Rewrite the parsing engine for speed (3.7x rows, 10.6x with the new iter_terms)

.github/workflows/run-unit-tests.yml

@@ -6,31 +6,34 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
-        python-version: ['3.7', '3.8', '3.9', '3.10', '3.11', '3.12', '3.13', 'pypy-3.9' ]
-        exclude: # Python < v3.8 does not support Apple Silicon ARM64.
-          - python-version: "3.7"
-            os: macos-latest
-        include: # So run those legacy versions on Intel CPUs.
-          - python-version: "3.7"
-            os: macos-13
+        python-version: ['3.8', '3.9', '3.10', '3.11', '3.12', '3.13', 'pypy-3.9']
     steps:
-      - uses: actions/checkout@v2
-      - uses: actions/setup-python@v2
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python-version }}
+          # setup-python caches pip itself, which replaces the manual actions/cache step this
+          # workflow used to carry. The dependency path is explicit because the default looks
+          # for requirements.txt, which this project does not have.
+          cache: pip
+          cache-dependency-path: |
+            requirements-dev.txt
+            setup.py
       - name: Upgrade pip
         run: python -m pip install --upgrade pip
-      - name: Get pip cache dir
-        id: pip-cache
-        run: |
-          echo "::set-output name=dir::$(python -m pip cache dir)"
-      - name: pip cache
-        uses: actions/cache@v2
-        with:
-          path: ${{ steps.pip-cache.outputs.dir }}
-          key: ${{ runner.os }}-${{ matrix.python-version }}-pip-${{ hashFiles('**/requirements.txt') }}
-          restore-keys: |
-            ${{ runner.os }}-${{ matrix.python-version }}-pip-
       - run: python -m pip install .
-      - run: pip install -r requirements-dev.txt
-      - run: pytest
+      - name: Install dev dependencies
+        shell: bash
+        run: |
+          if [[ "${{ matrix.python-version }}" == pypy* ]]; then
+            # pandas publishes no PyPy wheels, so pip falls back to building numpy from
+            # source and its C++ fails to compile on these runners. pandas is an optional
+            # dependency of this library and the tests that need it skip themselves, so PyPy
+            # runs the suite without it. That also gives the "pandas is not installed" code
+            # path the only CI coverage it has.
+            grep -v '^pandas' requirements-dev.txt > requirements-ci.txt
+            pip install -r requirements-ci.txt
+          else
+            pip install -r requirements-dev.txt
+          fi
+      - run: pytest

.gitignore

@@ -9,4 +9,5 @@ build/
 *.egg-info/
 .python-version
 .mypy_cache
-.tmp/
+.tmp/
+requirements-ci.txt

CHANGES.txt

@@ -1,3 +1,54 @@
+v0.17.0 (unreleased)
+--------------------
+
+- New: DwCAReader.iter_terms() and CSVDataFile.iter_terms() yield a tuple of values per row
+  for a chosen list of terms, skipping both the Row object and its data dict. Measured
+  roughly 2.9x faster than iterating over rows and reading the same terms, and roughly 10x
+  faster than the pre-rewrite implementation, on a 400000-row archive reading 14 of 50
+  columns, on one machine; see benchmarks/README.md for the full measurement.
+- New: iter_terms() accepts "id" and "coreid" to request the archive's key columns, which
+  often have no declared term of their own. These are the names headers() already uses.
+- Performance: iterating over an archive is a single streaming pass instead of one seek and
+  one throwaway csv.reader per row. Measured roughly 3.8-4.0x faster on a 400000-row GBIF
+  download on one machine; see benchmarks/README.md for the full before/after measurement.
+- Performance: the line offset index used for random access is now built on first use, so
+  opening an archive no longer scans every data file.
+- Fixed: DataFileDescriptor.headers dropped the column at index 0 for archives without a
+  metafile, which also made pd_read() promote that column to the DataFrame index.
+- Fixed: hash() on a CoreRow or an ExtensionRow raised TypeError. Rows have been documented
+  as hashable since 0.3.3 but never were.
+- Fixed: comparing or hashing a CoreRow obtained without linking to its DwCAReader (e.g. via
+  CSVDataFile.get_row_by_position() directly rather than by iterating the reader) raised
+  AttributeError instead of comparing successfully.
+- Fixed: rows read from two DwCAReader instances over the same archive never compared equal,
+  even with identical content. Row equality embeds the DataFileDescriptor, which had no
+  __eq__ and therefore compared by object identity. DataFileDescriptor now compares (and
+  hashes) by value: the data file layout it describes.
+- Fixed: comparing a CoreRow to an ExtensionRow (or either to a non-row object) raised
+  AttributeError instead of returning False.
+- Fixed: ignoreHeaderLines values above 1 only skipped a single line when iterating a
+  CSVDataFile directly, which let header lines leak into coreid_index and
+  orphaned_extension_rows.
+- Fixed: an undecodable byte in a data file desynchronised the line offset index, so rows
+  after it were silently truncated or rejected.
+- Fixed: a field whose content started or ended with the archive's fieldsEnclosedBy
+  character had that character stripped.
+- Fixed: DwCAReader was its own iterator, so nesting two loops over the same reader silently
+  ran the inner one once, and calling get_corerow_by_id() or get_corerow_by_position() from
+  inside a loop never terminated. Each iteration now gets its own iterator.
+- Fixed: in archives using fieldsEnclosedBy, a field containing the line terminator was read
+  as truncated at that terminator, by iteration and by random access alike, and the rows after
+  it were misaligned (often raising InvalidArchive on an unrelated row). Such archives are now
+  read correctly: the line offset index indexes CSV records rather than physical lines.
+- Changed: removed the undeclared typing_extensions dependency (dwca.star_record now uses
+  typing.Literal). Python 3.7, which reached end of life in June 2023, is no longer
+  supported; the minimum is now 3.8.
+- Changed: DwCAReader.next() now keeps an implicit iterator independent of any for loop over
+  the same reader, and starts a new pass after being exhausted. Iterate over the reader
+  instead; next() remains only for backwards compatibility.
+- Documentation: the tutorial and the GBIF results page still called get_row_by_index(),
+  removed in v0.15.0. They now use get_corerow_by_position().
+
 v0.16.4 (2024-10-18)
 --------------------
 

benchmarks/README.md

@@ -0,0 +1,189 @@
+# Benchmarks
+
+Not part of the test suite. Run manually before and after a change that claims a speedup.
+
+    PYTHONPATH=. .venv/bin/python benchmarks/generate_archive.py /tmp/dwca-bench 400000
+    PYTHONPATH=. .venv/bin/python benchmarks/bench_reader.py /tmp/dwca-bench
+
+`generate_archive.py` writes a GBIF-shaped archive: 50 columns, tab separated, no field
+enclosure, no header line. 400000 rows is roughly 250MB. Because `fieldsEnclosedBy=""`,
+only the unquoted parsing path (the `line.rstrip(...).split(...)` branch of
+`CSVDataFile._iter_field_lists()` in `dwca/files.py`) is exercised by these benchmarks - the
+quoted-field path is not measured here.
+
+`PYTHONPATH=.` is required because the scripts are run directly (not via `python -m`),
+so the repository root is not otherwise on `sys.path` and `import dwca` fails.
+
+The `peak=` column comes from `resource.getrusage(...).ru_maxrss`, which is a process-wide
+high-water mark, not a per-measurement figure: it never decreases within a run, so each
+`timed()` line reports the peak RSS seen so far across the whole process, including
+everything measured by earlier lines. Compare `peak=` across separate invocations of the
+script, not across lines of the same run.
+
+The "open archive" timing is the only one that isolates archive-opening cost: "iterate,
+..." and "random access, ..." both open a fresh `DwCAReader` (via `with DwCAReader(...)`)
+inside the timed region, so their reported time includes opening the archive (parsing the
+metafile and building the core file's line-offset index) on top of the operation the label
+describes.
+
+## Baseline
+
+Measured on:
+
+- Commit: `771593134599074cc4ff804790065c3519783e51` (branch `parsing-performance`)
+- Python: 3.12.0 (CPython)
+- Machine: MacBook Pro (Mac14,5, Apple Silicon, arm64), macOS 26.5
+
+Numbers are only comparable within one machine. Command:
+
+    PYTHONPATH=. .venv/bin/python benchmarks/generate_archive.py /tmp/dwca-bench 400000
+    PYTHONPATH=. .venv/bin/python benchmarks/bench_reader.py /tmp/dwca-bench
+
+Generator output:
+
+    wrote 400000 rows, 50 columns, 248MB to /tmp/dwca-bench
+
+Benchmark output (second of two consecutive runs; both runs agreed within about 5%):
+
+    archive: /tmp/dwca-bench
+      open archive                                    0.24s  n=occurrence.txt  peak=71MB
+      iterate, no field access                        9.81s  n=400000  peak=71MB
+      iterate + read 14 terms                        10.32s  n=400000  peak=71MB
+      random access, ~14k seeks                       0.59s  n=14286  peak=71MB
+
+First run, for comparison (same archive, same process type, run immediately before the one
+above):
+
+    archive: /tmp/dwca-bench
+      open archive                                    0.46s  n=occurrence.txt  peak=70MB
+      iterate, no field access                       10.61s  n=400000  peak=70MB
+      iterate + read 14 terms                        10.25s  n=400000  peak=70MB
+      random access, ~14k seeks                       0.61s  n=14286  peak=70MB
+
+The "no field access" and "read 14 terms" timings are close to each other in both runs.
+This is expected, not a bug: `CoreRow.data` is fully materialized (all columns split and
+decoded) when the row is constructed during iteration, so the benchmark's extra `.get()`
+calls on an already-built dict add only marginal cost on top of the row-parsing work that
+both variants pay. The gap between the two iterate variants is a better indicator of
+"reading fields" cost added ON TOP of parsing than a full picture of parsing cost itself,
+which the "no field access" line represents.
+
+## After the streaming engine
+
+Measured on:
+
+- Commit: `c48124009f02fa24b0d5b3f853036f7b8f302f0a` (branch `parsing-performance`)
+- Python: 3.12.0 (CPython)
+- Machine: MacBook Pro (Mac14,5, Apple Silicon, arm64), macOS 26.5
+
+The Phase 0 baseline above was recorded in a separate session. Machine variance between
+sessions has been observed to be as large as ~40 percent on identical code, so it is not a
+trustworthy comparison by itself. To get an honest pair, both sides below were re-measured
+back to back, in one sitting, on an otherwise idle machine: the Phase 0 starting commit
+(`fd829b6`, checked out into a scratch worktree) immediately followed by the current commit
+above, against the same generated archive.
+
+Generator output (shared by both sides):
+
+    wrote 400000 rows, 50 columns, 248MB to /tmp/dwca-bench
+
+Before (commit `fd829b61962b5a813705a0f7c5d1f12c1efe07e7`, run 1 of 2):
+
+    archive: /tmp/dwca-bench
+      open archive                                    0.23s  n=occurrence.txt  peak=73MB
+      iterate, no field access                        6.76s  n=400000  peak=73MB
+      iterate + read 14 terms                         6.84s  n=400000  peak=73MB
+      random access, every 7th row up to 100k         0.40s  n=14286  peak=73MB
+
+Before, run 2 of 2 (same archive, same process type, run immediately after):
+
+    archive: /tmp/dwca-bench
+      open archive                                    0.16s  n=occurrence.txt  peak=71MB
+      iterate, no field access                        6.80s  n=400000  peak=71MB
+      iterate + read 14 terms                         6.88s  n=400000  peak=71MB
+      random access, every 7th row up to 100k         0.38s  n=14286  peak=71MB
+
+After (commit `c48124009f02fa24b0d5b3f853036f7b8f302f0a`, run 1 of 2, measured immediately
+after the "before" runs, same archive):
+
+    archive: /tmp/dwca-bench
+      open archive                                    0.00s  n=occurrence.txt  peak=67MB
+      iterate, no field access                        1.65s  n=400000  peak=67MB
+      iterate + read 14 terms                         1.74s  n=400000  peak=67MB
+      random access, every 7th row up to 100k         0.26s  n=14286  peak=79MB
+
+After, run 2 of 2:
+
+    archive: /tmp/dwca-bench
+      open archive                                    0.00s  n=occurrence.txt  peak=67MB
+      iterate, no field access                        1.68s  n=400000  peak=67MB
+      iterate + read 14 terms                         1.79s  n=400000  peak=67MB
+      random access, every 7th row up to 100k         0.19s  n=14286  peak=76MB
+
+Both sides are consistent run to run (within a few percent). Using the average of the two
+runs on each side:
+
+- `open archive`: 0.20s -> 0.00s (below the timer's resolution; opening no longer scans the
+  data file to build the line offset index, it is now built lazily on first positional
+  access).
+- `iterate, no field access`: 6.78s -> 1.67s, roughly 4.1x faster.
+- `iterate + read 14 terms`: 6.86s -> 1.77s, roughly 3.9x faster (range 3.8x-4.0x across the
+  two run pairs). This is the headline number: it clears the phase's 2.5x target by a wide
+  margin.
+- `random access, every 7th row up to 100k`: 0.39s -> 0.23s, roughly 1.7x faster.
+
+`peak=` rose slightly on the "after" random access line (79MB / 76MB vs 67MB elsewhere in
+the same runs) because that is the first operation in the process that builds the line
+offset index; it remains well below the "before" side's peak, where the index was built
+eagerly on open.
+
+The absolute timings above are not comparable across sessions - only the ratio between two
+runs measured back to back in the same sitting is. Confirmed later: the same two commits
+(`fd829b6` and `c481240`) that recorded `iterate + read 14 terms` at 6.86s and 1.77s here
+measured 9.34s and 2.50s on a later, busier session on the same machine - the absolute
+numbers moved by roughly a third, but the ratio held (3.7x-4.0x measured back to back that
+time, against 3.8x-4.0x recorded above). Do not read the absolute seconds as a target or a
+regression signal in isolation; re-measure both sides back to back before drawing any
+conclusion from them.
+
+## iter_terms
+
+`DwCAReader.iter_terms()` / `CSVDataFile.iter_terms()` skip building both the `Row` object
+and its term-to-value dict, yielding a plain tuple of the requested terms per row instead.
+Measured against the row API on the same 400000-row, 50-column archive generated above
+(`/tmp/dwca-bench`), reading 14 terms per row in both cases, both paths measured back to
+back in one run (rerun twice to check consistency):
+
+    # Row path
+    with DwCAReader(ARCHIVE, skip_metadata=True) as dwca:
+        for row in dwca:
+            data = row.data
+            for term in TERMS:
+                data.get(term)
+
+    # iter_terms path
+    with DwCAReader(ARCHIVE, skip_metadata=True) as dwca:
+        for values in dwca.iter_terms(TERMS):
+            pass
+
+Run 1:
+
+    rows path      2.86s
+    iter_terms     0.98s
+    ratio (rows/iter_terms): 2.93x
+
+Run 2 (immediately after, same process type):
+
+    rows path      2.13s
+    iter_terms     0.75s
+    ratio (rows/iter_terms): 2.85x
+
+Run 3:
+
+    rows path      2.12s
+    iter_terms     0.72s
+    ratio (rows/iter_terms): 2.95x
+
+Consistent at roughly 2.85x-2.95x across three back-to-back runs, comfortably above the
+"roughly half the rows path" (2x) expectation. As with the numbers above, absolute seconds
+vary by session; only the ratio, measured back to back, is meaningful.

benchmarks/bench_reader.py

@@ -0,0 +1,87 @@
+"""Time the documented read paths against a generated archive.
+
+Usage:
+    python benchmarks/generate_archive.py /tmp/dwca-bench 400000
+    python benchmarks/bench_reader.py /tmp/dwca-bench
+
+Record the output before and after a change: this is the evidence for any speedup claim.
+"""
+
+import resource
+import sys
+import time
+
+from dwca.read import DwCAReader
+
+# A subset a real consumer reads, rather than every column.
+WANTED = [
+    "http://rs.tdwg.org/dwc/terms/" + name
+    for name in (
+        "occurrenceID scientificName basisOfRecord kingdom family genus country "
+        "locality decimalLatitude decimalLongitude year month day recordedBy"
+    ).split()
+]
+
+
+def peak_memory_mb():
+    usage = resource.getrusage(resource.RUSAGE_SELF).ru_maxrss
+    # Linux reports kilobytes, macOS reports bytes.
+    if sys.platform == "darwin":
+        usage = usage / 1024
+    return usage / 1024
+
+
+def timed(label, function):
+    started = time.perf_counter()
+    count = function()
+    elapsed = time.perf_counter() - started
+    print(
+        "  {label:44s} {elapsed:7.2f}s  n={count}  peak={memory:.0f}MB".format(
+            label=label, elapsed=elapsed, count=count, memory=peak_memory_mb()
+        )
+    )
+
+
+def main(archive_path):
+    def open_and_close():
+        reader = DwCAReader(archive_path, skip_metadata=True)
+        location = reader.core_file_location
+        reader.close()
+        return location
+
+    def iterate(read_fields):
+        count = 0
+        with DwCAReader(archive_path, skip_metadata=True) as reader:
+            for row in reader:
+                if read_fields:
+                    data = row.data
+                    for term in WANTED:
+                        data.get(term)
+                count += 1
+        return count
+
+    def random_access():
+        with DwCAReader(archive_path, skip_metadata=True) as reader:
+            data_file = reader.core_file
+            # No public API exposes the row count. _line_offsets is built lazily on first
+            # positional access, so we go through _get_line_offsets() (which builds it if
+            # needed) rather than reading the raw attribute directly. This lets the range
+            # below scale with the actual archive instead of hardcoding 100000 (which raises
+            # IndexError on smaller archives).
+            row_count = len(data_file._get_line_offsets()) - data_file.lines_to_ignore
+            upper_bound = min(row_count, 100000)
+            return sum(
+                1
+                for i in range(0, upper_bound, 7)
+                if data_file.get_row_by_position(i)
+            )
+
+    print("archive:", archive_path)
+    timed("open archive", open_and_close)
+    timed("iterate, no field access", lambda: iterate(False))
+    timed("iterate + read 14 terms", lambda: iterate(True))
+    timed("random access, every 7th row up to 100k", random_access)
+
+
+if __name__ == "__main__":
+    main(sys.argv[1])