diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index c0e415c..32db29d 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.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 \ No newline at end of file + - 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 diff --git a/.gitignore b/.gitignore index 4e72ff3..d672b17 100644 --- a/.gitignore +++ b/.gitignore @@ -9,4 +9,5 @@ build/ *.egg-info/ .python-version .mypy_cache -.tmp/ \ No newline at end of file +.tmp/ +requirements-ci.txt diff --git a/CHANGES.txt b/CHANGES.txt index 2f8749e..7a98d71 100644 --- a/CHANGES.txt +++ b/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) -------------------- diff --git a/benchmarks/README.md b/benchmarks/README.md new file mode 100644 index 0000000..5a7cac3 --- /dev/null +++ b/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. diff --git a/benchmarks/bench_reader.py b/benchmarks/bench_reader.py new file mode 100644 index 0000000..797b2e9 --- /dev/null +++ b/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]) diff --git a/benchmarks/generate_archive.py b/benchmarks/generate_archive.py new file mode 100644 index 0000000..0fb175b --- /dev/null +++ b/benchmarks/generate_archive.py @@ -0,0 +1,77 @@ +"""Generate a large, GBIF-shaped Darwin Core Archive for benchmarking. + +Usage: + python benchmarks/generate_archive.py /tmp/bench-archive 400000 +""" + +import os +import sys + +TERMS = ( + ["http://rs.tdwg.org/dwc/terms/occurrenceID"] + + ["http://rs.gbif.org/terms/1.0/gbifID"] + + [ + "http://rs.tdwg.org/dwc/terms/" + name + for name in ( + "datasetName basisOfRecord scientificName scientificNameAuthorship taxonID " + "kingdom phylum class order family genus specificEpithet taxonRank " + "countryCode country stateProvince county locality municipality " + "decimalLatitude decimalLongitude coordinateUncertaintyInMeters " + "verbatimLatitude verbatimLongitude minimumElevationInMeters " + "maximumElevationInMeters minimumDepthInMeters maximumDepthInMeters " + "year month day eventDate dateIdentified identifiedBy recordedBy " + "individualCount occurrenceStatus identificationVerificationStatus " + "institutionCode collectionCode catalogNumber datasetID references " + "continent habitat sex lifeStage establishmentMeans" + ).split() + ] +) + +METAFILE = """ + + occurrence.txt + +{fields} + + +""" + +SAMPLE = [ + "PRESERVED_SPECIMEN", "Chelonodon fluviatilis (Hamilton, 1822)", "Animalia", + "Chordata", "Actinopterygii", "Tetraodontiformes", "Tetraodontidae", "Tetraodon", + "India", "Andaman and Nicobar Islands", "Port Blair", "11.6234", "92.7265", + "1930", "4", "1", "Misra, K. S.; Rao, H. Srinivasa", "CAS", "SU (ICH)", +] + + +def main(directory, row_count): + os.makedirs(directory, exist_ok=True) + + fields = "\n".join( + ' '.format(i=i, t=term) + for i, term in enumerate(TERMS) + ) + with open(os.path.join(directory, "meta.xml"), "w", encoding="utf-8") as f: + f.write(METAFILE.format(fields=fields)) + + column_count = len(TERMS) + with open( + os.path.join(directory, "occurrence.txt"), "w", encoding="utf-8", newline="" + ) as f: + for i in range(row_count): + cells = [str(600000000 + i), str(900000000 + i)] + while len(cells) < column_count: + cells.append(SAMPLE[len(cells) % len(SAMPLE)]) + f.write("\t".join(cells[:column_count]) + "\n") + + size = os.path.getsize(os.path.join(directory, "occurrence.txt")) + print( + "wrote {n} rows, {c} columns, {mb:.0f}MB to {d}".format( + n=row_count, c=column_count, mb=size / (1024 * 1024), d=directory + ) + ) + + +if __name__ == "__main__": + main(sys.argv[1], int(sys.argv[2])) diff --git a/doc/api.rst b/doc/api.rst index 02b98cb..2501151 100644 --- a/doc/api.rst +++ b/doc/api.rst @@ -28,6 +28,12 @@ File objects .. automodule:: dwca.files :members: +Star record objects +------------------- + +.. automodule:: dwca.star_record + :members: + Helpers ------- diff --git a/doc/gbif_results.rst b/doc/gbif_results.rst index b5e115d..2c96379 100644 --- a/doc/gbif_results.rst +++ b/doc/gbif_results.rst @@ -29,7 +29,7 @@ You can access this source metadata like this: # 'dataset2_UUID': , ...} # 2. From a CoreRow instance, we can get back to the metadata of its source dataset: - first_row = results.get_row_by_index(0) + first_row = results.get_corerow_by_position(0) print(first_row.source_metadata) # => @@ -45,7 +45,7 @@ Because there's a standard core-extension relationship (star schema) between tho from dwca.read import DwCAReader with DwCAReader('gbif-results.zip') as results: - first_row = results.get_row_by_index(0) + first_row = results.get_corerow_by_position(0) first_row.extensions diff --git a/doc/index.rst b/doc/index.rst index 2a2e3d7..2fb3f48 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -13,7 +13,7 @@ Archives can be enclosed in either a directory or a zip/tgz archive. It supports most common features from the Darwin Core Archive standard, including extensions and `Simple Darwin Core`_ expressed as text (aka Archives consisting of a single CSV data file, possibly with Metadata but without Metafile). -It officially supports Python 3.5+ and has been reported to work on Jython by at least one user. It works on Linux, Mac OS and since v0.10.2 also on Windows. +It officially supports Python 3.8+ and has been reported to work on Jython by at least one user. It works on Linux, Mac OS and since v0.10.2 also on Windows. Use version 0.13.2 if you need Python 2.7 support. Status diff --git a/doc/tutorial.rst b/doc/tutorial.rst index 2be852e..5ae60e9 100644 --- a/doc/tutorial.rst +++ b/doc/tutorial.rst @@ -80,7 +80,7 @@ Basic use, access to metadata and data from the Core file # guarantee unicity (nor even that there will be an id). The index (position) of the row (starting at 0) is # generally preferable. - occurrence_on_second_line = dwca.get_row_by_index(1) + occurrence_on_second_line = dwca.get_corerow_by_position(1) # We can retreive the (absolute) of embedded files # NOTE: this path point to a temporary directory that will be removed at the end of the DwCAReader object life @@ -88,6 +88,51 @@ Basic use, access to metadata and data from the Core file path = dwca.absolute_temporary_path('occurrence.txt') +Reading only a few terms +~~~~~~~~~~~~~~~~~~~~~~~~ + +If you only need a handful of terms and the archive is large, :meth:`~dwca.read.DwCAReader.iter_terms` +is roughly twice as fast as iterating over rows: it yields a plain tuple per row and never +builds a :class:`~dwca.rows.CoreRow` object nor its ``data`` dictionary. + +.. code:: python + + from dwca.read import DwCAReader + from dwca.darwincore.utils import qualname as qn + + with DwCAReader('my-archive.zip') as dwca: + for locality, family, scientific_name in dwca.iter_terms( + [qn('locality'), qn('family'), qn('scientificName')]): + print(locality, family, scientific_name) + +Values are returned in the order the terms were requested, and they are normalised exactly as +``row.data`` would be: an empty cell falls back to the term's default value from the Metafile, +or to an empty string. Requesting a term the data file does not contain raises ``ValueError``. + +A single term still yields a one-element tuple: + +.. code:: python + + for (locality,) in dwca.iter_terms([qn('locality')]): + print(locality) + +The special name ``"id"`` requests the core file's id column - the same name used by +:attr:`~dwca.descriptors.DataFileDescriptor.headers`. It can be mixed freely with regular +terms, and it resolves even when the Metafile declares no ```` for that column, which +is the common case: + +.. code:: python + + for identifier, family in dwca.iter_terms(['id', qn('family')]): + print(identifier, family) + +If the Metafile happens to declare a field literally named ``"id"`` (this occurs with +metafile-less archives, where terms are the raw CSV header names), that declared term takes +precedence over the id column, matching what ``row.data['id']`` already returns. The same +applies to ``"coreid"`` when calling :meth:`~dwca.files.CSVDataFile.iter_terms` on an +extension file (``dwca.extension_files[i]``) instead of on the reader. + + Access to Darwin Core Archives with extensions (star schema) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/decisions.md b/docs/decisions.md new file mode 100644 index 0000000..21ff83b --- /dev/null +++ b/docs/decisions.md @@ -0,0 +1,11 @@ +# Decisions + +## 2026-07-28 - Rows and descriptors compare by value + +**What:** `DataFileDescriptor` got `__eq__`/`__hash__` over the data file layout it describes, +so rows from two readers over the same archive now compare equal. +**Why:** Row equality embeds the descriptor; with identity-based comparison, rows identical in +data, raw_fields, id, position and rowtype still compared unequal across readers. +**Rejected:** Documenting the single-reader scope on `CoreRow.__eq__` instead - a descriptor +describes a file layout, not the reader that built it, so identity comparison was a bug rather +than a boundary worth keeping. diff --git a/dwca/descriptors.py b/dwca/descriptors.py index 5623669..61fe926 100644 --- a/dwca/descriptors.py +++ b/dwca/descriptors.py @@ -12,6 +12,7 @@ import io import os import re +from operator import itemgetter import xml.etree.ElementTree as ET from typing import Optional, List, Dict, Set from xml.etree.ElementTree import Element @@ -210,6 +211,66 @@ def make_from_metafile_section(cls, section_tag): fields_terminated_by=fields_terminated_by, ) + def __key(self): + """Return a tuple describing the data file layout. Common ground between equality and hash. + + Deliberately excludes `raw_element`: it's an xml.etree.ElementTree.Element, which + compares by identity, so including it would make two descriptors built from the same + metafile section never compare equal. `lines_to_ignore` stands in for the only part of + `raw_element` that affects parsing (the ignoreHeaderLines attribute). + + `created_from_file` is excluded too: it records where the descriptor came from, not how + the file is laid out, and its only behavioral effect is already covered by + `lines_to_ignore`. `represents_extension` is excluded as it's just the negation of + `represents_corefile`. + """ + return ( + self.file_location, + self.file_encoding, + self.type, + self.id_index, + self.coreid_index, + self.represents_corefile, + self.fields, + self.lines_terminated_by, + self.fields_enclosed_by, + self.fields_terminated_by, + self.lines_to_ignore, + ) + + def __eq__(self, other): + if not isinstance(other, DataFileDescriptor): + return NotImplemented + + return self.__key() == other.__key() + + def __hash__(self): + # __key() embeds `fields`, a list of dicts, which isn't hashable. Equal descriptors + # still hash equally because this is a subset of the equality key. + return hash( + ( + self.file_location, + self.file_encoding, + self.type, + self.id_index, + self.coreid_index, + self.represents_corefile, + self.lines_terminated_by, + self.fields_enclosed_by, + self.fields_terminated_by, + self.lines_to_ignore, + ) + ) + + @property + def field_plan(self) -> "FieldPlan": + """A cached :class:`FieldPlan` turning a split data row into a term -> value dict.""" + plan = self.__dict__.get("_field_plan") + if plan is None: + plan = FieldPlan(self.fields, self.id_index, self.coreid_index) + self.__dict__["_field_plan"] = plan + return plan + @property def terms(self) -> Set[str]: """Return a Python set containing all the Darwin Core terms appearing in file.""" @@ -229,9 +290,9 @@ def headers(self) -> List[str]: columns = {} for f in self.fields: - if f[ - "index" - ]: # Some (default values for example) don't have a corresponding col. + # Some fields (those carrying only a default value) have no column. Note the + # explicit None test: index 0 is a valid column and must not be dropped. + if f["index"] is not None: columns[f["index"]] = f["term"] # In addition to DwC terms, we may also have id (Core) or core_id (Extensions) columns @@ -264,6 +325,181 @@ def lines_to_ignore(self) -> int: return int(self.raw_element.get("ignoreHeaderLines", 0)) +class FieldPlan(object): + """Precomputed mapping from a split CSV line to a term -> value dict. + + Built once per :class:`DataFileDescriptor` (see its ``field_plan`` property) so that + parsing a row costs one C-level ``dict(zip(...))`` instead of a Python loop over the + field descriptors. + """ + + __slots__ = ( + "_fields", + "_id_index", + "_coreid_index", + "_contiguous_terms", + "_terms", + "_getter", + "_single_column", + "_defaults", + "_constants", + "_ordered", + "required_columns", + ) + + def __init__(self, fields, id_index=None, coreid_index=None): + self._fields = fields + self._id_index = id_index + self._coreid_index = coreid_index + + indexed = [f for f in fields if f["index"] is not None] + indexes = [f["index"] for f in indexed] + + #: Number of columns a data row must have for this plan to apply. + self.required_columns = max(indexes) + 1 if indexes else 0 + + # Terms without a column: the value is always the default. + self._constants = tuple( + (f["term"], f["default"] or "") for f in fields if f["index"] is None + ) + # Indexed terms that also carry a default, used when the cell is empty (issue #80). + self._defaults = tuple( + (f["term"], f["default"]) for f in indexed if f["default"] + ) + + # When some terms have no column, the fast paths below would append them after the + # mapped ones and change the key order of Row.data (which is user-visible through + # str(row)). Those archives use an ordered path instead; they are rare, and the + # ordered path is still free of the per-field try/except and int() it replaces. + self._ordered = ( + tuple((f["term"], f["index"], f["default"]) for f in fields) + if self._constants + else None + ) + + if indexes and sorted(indexes) == list(range(len(indexes))): + # Fast path: the indexed fields cover columns 0..n-1 exactly once. + ordered = sorted(indexed, key=lambda f: f["index"]) + self._contiguous_terms = tuple(f["term"] for f in ordered) + self._terms = None + self._getter = None + self._single_column = False + else: + self._contiguous_terms = None + self._terms = tuple(f["term"] for f in indexed) + self._getter = itemgetter(*indexes) if indexes else None + # itemgetter with a single argument returns a scalar, not a tuple. + self._single_column = len(indexes) == 1 + + def build_data(self, raw_fields): + """Return the term -> value dict for an already-split data row.""" + if len(raw_fields) < self.required_columns: + self._raise_missing_column(raw_fields) + + if self._ordered is not None: + data = {} + for term, index, default in self._ordered: + value = raw_fields[index] if index is not None else None + data[term] = value or default or "" + return data + + if self._contiguous_terms is not None: + data = dict(zip(self._contiguous_terms, raw_fields)) + elif self._getter is not None: + values = self._getter(raw_fields) + if self._single_column: + values = (values,) + data = dict(zip(self._terms, values)) + else: + data = {} + + for term, default in self._defaults: + if not data[term]: + data[term] = default + for term, constant in self._constants: + data[term] = constant + + return data + + def term_getter(self, terms): + """Return a callable mapping a split data row to a tuple of values for `terms`. + + :raises ValueError: if any requested term is absent from the data file. + """ + # `terms` is iterated three times below (missing, indexes, defaults). A generator or + # other one-shot iterable would be exhausted after the first pass, silently turning + # every later pass empty rather than raising - so normalise to a list once up front. + terms = list(terms) + + by_term = {f["term"]: f for f in self._fields} + + # "id" and "coreid" name the archive's key columns. These are the names this library + # already uses for them in headers(), short_headers() and pd_read(). A declared term of + # the same name wins, which matters for metafile-less archives whose terms are raw CSV + # header names, and keeps this consistent with row.data. + if "id" not in by_term and self._id_index is not None: + by_term["id"] = {"term": "id", "index": self._id_index, "default": None} + if "coreid" not in by_term and self._coreid_index is not None: + by_term["coreid"] = {"term": "coreid", "index": self._coreid_index, "default": None} + + missing = [term for term in terms if term not in by_term] + if missing: + raise ValueError( + "These terms are not in this data file: {t}".format( + t=", ".join(sorted(missing)) + ) + ) + + indexes = tuple(by_term[term]["index"] for term in terms) + defaults = tuple(by_term[term]["default"] for term in terms) + + if indexes and all(i is not None for i in indexes) and not any(defaults): + # Fast path: every term maps to a column and none has a default, so the whole + # tuple comes out of a single C-level call. + getter = itemgetter(*indexes) + required = max(indexes) + 1 + + if len(indexes) == 1: + # itemgetter with a single argument returns a scalar, not a tuple. + def get_one(raw_fields): + if len(raw_fields) < required: + self._raise_missing_column(raw_fields) + return (getter(raw_fields),) + + return get_one + + def get_many(raw_fields): + if len(raw_fields) < required: + self._raise_missing_column(raw_fields) + return getter(raw_fields) + + return get_many + + required = max([i for i in indexes if i is not None] or [-1]) + 1 + + def get_general(raw_fields): + if len(raw_fields) < required: + self._raise_missing_column(raw_fields) + return tuple( + (raw_fields[index] if index is not None else None) or default or "" + for index, default in zip(indexes, defaults) + ) + + return get_general + + def _raise_missing_column(self, raw_fields): + # Slow path: report the same index the old per-field loop would have reported. + for field in self._fields: + index = field["index"] + if index is not None and index >= len(raw_fields): + raise InvalidArchive( + "The descriptor references a non-existent field (index={i})".format( + i=index + ) + ) + raise InvalidArchive("The descriptor references a non-existent field") + + class ArchiveDescriptor(object): """Class used to encapsulate the whole Metafile (`meta.xml`).""" diff --git a/dwca/files.py b/dwca/files.py index e91266c..1d83bb9 100644 --- a/dwca/files.py +++ b/dwca/files.py @@ -1,9 +1,11 @@ """File-related classes and functions.""" +import csv import io import os from array import array -from typing import List, Union, IO, Dict, Optional +from itertools import islice +from typing import Iterator, List, Tuple, Union, IO, Dict, Optional from dwca.descriptors import DataFileDescriptor from dwca.rows import CoreRow, ExtensionRow, Row @@ -25,11 +27,11 @@ class CSVDataFile(object): * For an extension data file, with :meth:`get_all_rows_by_coreid` (A :class:`dwca.rows.CoreRow` or \ :class:`dwca.rows.ExtensionRow` object is returned) - On initialization, an index of new lines is build. This may take time, but makes random access\ - faster. + An index of line offsets is built on first random access (:meth:`get_row_by_position` or\ + :meth:`get_all_rows_by_coreid`). This may take time for large files, but makes further random\ + access faster. """ - # TODO: More tests for this class def __init__( self, work_directory: str, file_descriptor: DataFileDescriptor ) -> None: @@ -38,32 +40,142 @@ def __init__( #: constructor. self.file_descriptor = file_descriptor # type: DataFileDescriptor - self._file_stream = io.open( - os.path.join(work_directory, self.file_descriptor.file_location), - mode="r", - encoding=self.file_descriptor.file_encoding, - newline=self.file_descriptor.lines_terminated_by, - errors="replace", + self._file_path = os.path.join( + work_directory, self.file_descriptor.file_location ) - - # On init, we parse the file once to build an index of newlines (including lines to ignore) - # that will make random access faster later on... - self._line_offsets = _get_all_line_offsets( - self._file_stream, self.file_descriptor.file_encoding + self._file_stream = self._open_stream( + newline=self.file_descriptor.lines_terminated_by ) + # Opened lazily, on first random-access read: a dedicated binary stream used to + # fetch the exact byte range of a record (see _read_record). + self._binary_stream = None # type: Optional[IO] + + # The index of line offsets is only needed for random access, so it is built on + # first use rather than here: opening an archive stays O(1) whatever its size. + self._line_offsets = None # type: Optional[array] + #: Number of lines to ignore (header lines) in the CSV file. self.lines_to_ignore = self.file_descriptor.lines_to_ignore # type: int self._coreid_index = None # type: Optional[Dict[str, List[int]]] + def iter_terms(self, terms: List[str]) -> Iterator[Tuple[str, ...]]: + """Yield one tuple of values per data row, holding `terms` in the order given. + + This is a faster alternative to iterating over rows for consumers that only need a + few of the file's columns: neither a :class:`dwca.rows.Row` object nor its term to + value dict is built. A single term still yields a one-element tuple. + + Usage:: + + for locality, latitude in data_file.iter_terms([qn('locality'), + qn('decimalLatitude')]): + pass + + The special names "id" and "coreid" request the file's key column - the same names + used by :attr:`dwca.descriptors.DataFileDescriptor.headers`. "id" resolves for a core + file, "coreid" for an extension file; the other one raises, as does either name when + the file has no such column (metafile-less archives have neither). If the file + declares an actual term of the same name, that declared term takes precedence, which + matches what `Row.data['id']` already returns. + + :param terms: a list of full term identifiers. + :raises ValueError: if any of `terms` is not present in this data file. + """ + # The getter is resolved eagerly so an unknown term is reported by this call rather + # than on first iteration. + getter = self.file_descriptor.field_plan.term_getter(terms) + + return self._iter_terms(getter) + + def _iter_terms(self, getter) -> Iterator[tuple]: + for fields in self._iter_field_lists(): + yield getter(fields) + def __str__(self) -> str: return self.file_descriptor.file_location + def _open_stream(self, newline: str) -> IO: + return io.open( + self._file_path, + mode="r", + encoding=self.file_descriptor.file_encoding, + newline=newline, + errors="replace", + ) + + def _get_line_offsets(self) -> array: + if self._line_offsets is None: + descriptor = self.file_descriptor + self._line_offsets = _build_line_offsets( + self._file_path, + descriptor.file_encoding, + descriptor.lines_terminated_by, + descriptor.fields_enclosed_by, + descriptor.fields_terminated_by, + ) + return self._line_offsets + + def _iter_field_lists(self) -> Iterator[List[str]]: + """Yield each data row of the file as a list of raw field values. + + Header lines are skipped. This is a single forward pass over a dedicated stream, so + it is safe to run several of these concurrently and alongside random access. + """ + if self._file_stream.closed: + raise ValueError("The data file has been closed.") + + descriptor = self.file_descriptor + quoted = descriptor.fields_enclosed_by != "" + + # The csv module refuses a stream that can hand it an embedded carriage return, so + # the quoted path has to let Python handle newlines. The unquoted path keeps the + # archive's own terminator, which is what stops U+0085 from being treated as a line + # break (issue #20). + stream = self._open_stream( + newline="" if quoted else descriptor.lines_terminated_by + ) + try: + if quoted: + source = csv.reader( + stream, + delimiter=descriptor.fields_terminated_by, + quotechar=descriptor.fields_enclosed_by, + quoting=csv.QUOTE_MINIMAL, + ) # type: Iterator[List[str]] + else: + # A carriage return is stripped alongside the declared terminator: archives + # routinely declare "\n" while the file itself has CRLF line endings, and the + # csv module this replaced dropped the stray CR for us. Without this, the last + # field of every row would keep it. + line_ending = descriptor.lines_terminated_by + "\r" + separator = descriptor.fields_terminated_by + source = (line.rstrip(line_ending).split(separator) for line in stream) + + for fields in islice(source, self.lines_to_ignore, None): + yield fields + finally: + stream.close() + + def iter_rows(self) -> Iterator[Union[CoreRow, ExtensionRow]]: + """Yield every data row of the file, in order of appearance. + + Unlike repeated :meth:`get_row_by_position` calls this is a single forward pass and + never touches the line offset index. + """ + descriptor = self.file_descriptor + row_class = CoreRow if descriptor.represents_corefile else ExtensionRow + + for position, fields in enumerate(self._iter_field_lists()): + yield row_class.from_fields(fields, position, descriptor) + def _position_file_after_header(self) -> None: self._file_stream.seek(0, 0) - if self.lines_to_ignore > 0: - self._file_stream.readlines(self.lines_to_ignore) + # NOTE: readlines() takes a byte-size hint, not a line count, so it cannot be used + # here. With ignoreHeaderLines="2" it would read a single line. + for _ in range(self.lines_to_ignore): + self._file_stream.readline() def __iter__(self) -> "CSVDataFile": self._position_file_after_header() @@ -73,6 +185,12 @@ def __next__(self) -> str: return self.next() def next(self) -> str: # NOQA + """Return the next raw line of the data file, including its line terminator. + + Header lines are skipped. Raises `StopIteration` once the file is exhausted. This is + the iterator protocol behind `for line in data_file`, described in the class + docstring. + """ for line in self._file_stream: return line @@ -108,20 +226,22 @@ def _build_coreid_index(self) -> Dict[str, List[int]]: """Build and return an index of Core Rows IDs suitable for `CSVDataFile.coreid_index`.""" index = {} # type: Dict[str, array[int]] - for position, row in enumerate(self): - if self.file_descriptor.represents_corefile: - tmp = CoreRow(row, position, self.file_descriptor) - index.setdefault(tmp.id, array("L")).append(position) - else: - tmp = ExtensionRow(row, position, self.file_descriptor) - index.setdefault(tmp.core_id, array("L")).append(position) + represents_corefile = self.file_descriptor.represents_corefile + for row in self.iter_rows(): + key = row.id if represents_corefile else row.core_id + index.setdefault(key, array("L")).append(row.position) return index - # TODO: For ExtensionRow and a specific field only, generalize ? - # TODO: What happens if called on a Core Row? def get_all_rows_by_coreid(self, core_id: int) -> List[Row]: - """Return a list of :class:`dwca.rows.ExtensionRow` whose Core Id field match `core_id`.""" + """Return the rows whose linking id matches `core_id`. + + For an extension file, this is the row's `coreid` field. For a core file, it is the + row's own `id`, since `coreid_index` then maps each row's id to its position. The + return type is `List[Row]` to cover both :class:`dwca.rows.CoreRow` (core file) and + :class:`dwca.rows.ExtensionRow` (extension file). Returns an empty list if `core_id` + is not found. + """ if core_id not in self.coreid_index: return [] @@ -143,8 +263,48 @@ def get_row_by_position(self, position: int) -> Union[CoreRow, ExtensionRow]: # Raises IndexError if position is incorrect def _get_line_by_position(self, position: int) -> str: - self._file_stream.seek(self._line_offsets[position + self.lines_to_ignore], 0) - return self._file_stream.readline() + if self._file_stream.closed: + raise ValueError("The data file has been closed.") + + offsets = self._get_line_offsets() + index = position + self.lines_to_ignore + return self._read_record(offsets, index) + + def _get_binary_stream(self) -> IO: + # A dedicated handle, separate from self._file_stream: random access reads the + # exact byte range of a record (see _read_record), and must not disturb the + # position of the text stream used for sequential iteration. + if self._binary_stream is None: + self._binary_stream = io.open(self._file_path, mode="rb") + return self._binary_stream + + def _read_record(self, offsets: array, index: int) -> str: + """Return the full text of the record starting at offsets[index]. + + offsets holds byte offsets, computed by scanning the file as bytes (see + _build_line_offsets). A text stream's read(n) counts characters, not bytes, so + turning a byte delta into a character count would silently truncate or over-read + wherever a character takes more than one byte in the file's encoding (e.g. + multi-byte UTF-8). Reading the exact byte range through a dedicated binary stream + and decoding it afterwards keeps this correct regardless of encoding, and also + regardless of how many physical lines the record spans (a quoted field may + legally contain the line terminator). + + `index` supports the same negative-indexing quirk as a plain list/array + (get_row_by_position(-1) is pinned by TestNegativePosition), so `offsets[index]` + is used rather than any manual wraparound arithmetic. + """ + start = offsets[index] + stream = self._get_binary_stream() + stream.seek(start, 0) + + following = index + 1 + if following < len(offsets) and offsets[following] > start: + raw = stream.read(offsets[following] - start) + else: + raw = stream.read() + + return raw.decode(self.file_descriptor.file_encoding, errors="replace") def close(self) -> None: """Close the file. @@ -152,30 +312,196 @@ def close(self) -> None: The content of the file will not be accessible in any way afterwards. """ self._file_stream.close() - - -def _get_all_line_offsets(f: IO, encoding: str) -> array: - """Parse the file whose handler is given and return an array (long) containing the start offset\ - of each line. - - The values in the array are suitable for seek() operations. - - This function can take long for large files. - - It needs to know the file encoding to properly count the bytes in a given string. + if self._binary_stream is not None: + self._binary_stream.close() + + +def _build_line_offsets( + path: str, + encoding: str, + lines_terminated_by: str, + fields_enclosed_by: str = "", + fields_terminated_by: str = "\t", + chunk_size: int = 1024 * 1024, +) -> array: + """Return an array of the byte offset of every CSV record in the file at `path`. + + The values are suitable for seek() on a stream opened with the same encoding. + + Without an enclosure character every terminator ends a record, so this is a plain scan. + With one, a terminator inside an enclosed field is data rather than a record boundary + (a quoted field may legally contain the line terminator), so the scan tracks whether it + is inside an enclosure. + + The file is scanned as bytes rather than as decoded text: decoding with + errors="replace" turns an undecodable byte into U+FFFD, which does not re-encode to the + same length, so computing offsets from decoded text desynchronises the index from the + file. + + An array of Longs is used instead of a list. It is much more memory efficient, and a few + tests with 1-4Gb uncompressed archives didn't show any significant slowdown. """ - f.seek(0, 0) - - # We use an array of Longs instead of a list to store the index. - # It's much more memory efficient, and a few tests w/ 1-4Gb uncompressed archives - # didn't show any significant slowdown. - # - # See mini-benchmark in minibench.py - line_offsets = array("L") - offset = 0 - for line in f: - line_offsets.append(offset) - offset += len(line.encode(encoding)) - - f.seek(0, 0) - return line_offsets + terminator = lines_terminated_by.encode(encoding) + tlen = len(terminator) + + if not fields_enclosed_by: + return _scan_plain(path, terminator, tlen, chunk_size) + + quote = fields_enclosed_by.encode(encoding) + + # Fast screen: if no physical line ends with an unbalanced number of quote characters, + # then no record spans a line break and the plain scan is already correct. This is the + # overwhelmingly common case - including files where EVERY field is quoted - and it costs + # one C-level count() per line instead of a Python step per quote character. + offsets = _scan_screened(path, terminator, tlen, quote, chunk_size) + if offsets is not None: + return offsets + + return _scan_enclosed( + path, + terminator, + tlen, + quote, + fields_terminated_by.encode(encoding), + chunk_size, + ) + + +def _scan_plain(path, terminator, tlen, chunk_size): + offsets = array("L", [0]) + buffer = b"" + base = 0 + with io.open(path, "rb") as f: + while True: + chunk = f.read(chunk_size) + if not chunk: + break + buffer += chunk + consumed = 0 + while True: + found = buffer.find(terminator, consumed) + if found == -1: + break + consumed = found + tlen + offsets.append(base + consumed) + base += consumed + buffer = buffer[consumed:] + _trim(offsets, path) + return offsets + + +def _scan_screened(path, terminator, tlen, quote, chunk_size): + """Return record offsets if every physical line has balanced quotes, else None.""" + offsets = array("L", [0]) + buffer = b"" + base = 0 + line_start = 0 # index within buffer of the current line's first byte + with io.open(path, "rb") as f: + while True: + chunk = f.read(chunk_size) + if not chunk: + break + buffer += chunk + consumed = 0 + while True: + found = buffer.find(terminator, consumed) + if found == -1: + break + if buffer.count(quote, line_start, found) % 2: + return None # a quoted field spans this line break; needs the exact scan + consumed = found + tlen + line_start = consumed + offsets.append(base + consumed) + base += consumed + buffer = buffer[consumed:] + line_start = 0 + _trim(offsets, path) + return offsets + + +def _scan_enclosed(path, terminator, tlen, quote, delimiter, chunk_size): + qlen = len(quote) + dlen = len(delimiter) + # A quote opens a field only at the very start of a record or immediately after a + # delimiter; anywhere else it is literal content, which is what csv.reader does under + # QUOTE_MINIMAL. Deciding that needs to look back at the bytes before the quote, and + # deciding whether a quote is escaped needs to look ahead, so the buffer keeps a margin + # of context on both sides of the cursor rather than being trimmed flush to it. + margin = max(dlen, tlen, 2 * qlen) + + offsets = array("L", [0]) + buffer = b"" + base = 0 # absolute offset of buffer[0] + pos = 0 # cursor within buffer + in_quotes = False + record_start = 0 # absolute offset of the current record + + with io.open(path, "rb") as f: + while True: + chunk = f.read(chunk_size) + eof = not chunk + if not eof: + buffer += chunk + + # Without more bytes coming, patterns can be resolved right up to the end. + limit = len(buffer) if eof else len(buffer) - margin + + while pos < len(buffer): + if in_quotes: + found = buffer.find(quote, pos) + if found == -1 or (not eof and found > limit): + pos = max(pos, limit) + break + if not eof and found + 2 * qlen > len(buffer): + break # cannot yet tell an escaped quote from a closing one + if buffer.startswith(quote, found + qlen): + pos = found + 2 * qlen + continue + in_quotes = False + pos = found + qlen + continue + + nq = buffer.find(quote, pos) + nt = buffer.find(terminator, pos) + if nt == -1 and nq == -1: + pos = max(pos, limit) + break + if not eof and min(x for x in (nq, nt) if x != -1) > limit: + pos = max(pos, limit) + break + if nt != -1 and (nq == -1 or nt < nq): + pos = nt + tlen + offsets.append(base + pos) + record_start = base + pos + continue + if base + nq == record_start: + opens = True + else: + opens = ( + (nq >= dlen and buffer.startswith(delimiter, nq - dlen)) + or (nq >= tlen and buffer.startswith(terminator, nq - tlen)) + ) + if opens: + in_quotes = True + pos = nq + qlen + + if eof: + break + + # Keep `margin` bytes behind the cursor so the look-back above still works after + # the buffer is trimmed. + drop = max(0, pos - margin) + buffer = buffer[drop:] + base += drop + pos -= drop + + _trim(offsets, path) + return offsets + + +def _trim(offsets: array, path: str) -> None: + # A file ending with the terminator has no extra empty line after it, so the offset + # pointing at EOF is spurious. This also empties the index for a zero-byte file, which + # must report no lines at all rather than one empty one. + if offsets and offsets[-1] == os.path.getsize(path): + offsets.pop() diff --git a/dwca/minibench.py b/dwca/minibench.py deleted file mode 100644 index 81d3bf2..0000000 --- a/dwca/minibench.py +++ /dev/null @@ -1,61 +0,0 @@ -# Quick'n'dirty mini benchmark used to compare memory and time performance of -# array() vs standard list for the _line_offsets attribute of class CSVDataFile -# -# Early 2015 results: array is much more efficient in term of memory, and doesn't -# seem slower => array wins. -import resource - -from dwca.read import DwCAReader - - -def sizeof_fmt(num, suffix="B"): - for unit in ["", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi"]: - if abs(num) < 1024.0: - return "%3.1f%s%s" % (num, unit, suffix) - num /= 1024.0 - return "%.1f%s%s" % (num, "Yi", suffix) - - -def show_memory_usage(): - bytes = resource.getrusage(resource.RUSAGE_SELF).ru_maxrss - print(sizeof_fmt(bytes)) - - -# print "Time before: " -# print time.ctime() - -# print "Memory before:" -# show_memory_usage() - - -def test(): - with DwCAReader("dwca-florabank1-occurrences") as dwca: - # print "Time after open:" - # print time.ctime() - - # print "Memory after open:" - # show_memory_usage() - - i = 0 - for row in dwca: - # tmp = row.data[qn('locality')] - i = i + 1 - if i % 100000 == 0: - print("in loop mem: ") - show_memory_usage() - - # print "Time after loop" - # print time.ctime() - - # print "Memory after loop:" - # show_memory_usage() - - # print "Memory at the end:" - # show_memory_usage() - - -if __name__ == "__main__": - from timeit import Timer - - t = Timer("test()", "from __main__ import test") - print(t.timeit(number=3)) diff --git a/dwca/read.py b/dwca/read.py index f4a152e..aeb53bd 100644 --- a/dwca/read.py +++ b/dwca/read.py @@ -7,7 +7,7 @@ import zipfile from errno import ENOENT from tempfile import mkdtemp -from typing import List, Optional, Dict, Any, IO, Tuple +from typing import Iterator, List, Optional, Dict, Any, IO, Tuple from xml.etree.ElementTree import Element import dwca.vendor @@ -38,6 +38,10 @@ class DwCAReader(object): :param tmp_dir: temporary directory to use to uncompress the archive (if needed). If not provided, Python default \ will be used. :type tmp_dir: str + :param skip_metadata: if `True`, the archive's scientific metadata file is not parsed and the `metadata` \ + attribute stays `None`. This does not affect `source_metadata`, which is still populated either way. Use this \ + to avoid the parsing cost when only the data rows are needed. + :type skip_metadata: bool :raises: :class:`dwca.exceptions.InvalidArchive` :raises: :class:`dwca.exceptions.InvalidSimpleArchive` @@ -100,6 +104,7 @@ def __init__( #: The path to the Darwin Core Archive file, as passed to the constructor. self.archive_path = path # type: str + self._default_iterator = None # type: Optional[Iterator[CoreRow]] if os.path.isdir( self.archive_path @@ -256,6 +261,30 @@ def pd_read(self, relative_path, **kwargs): return df_or_textreader + def iter_terms(self, terms: List[str]) -> Iterator[Tuple[str, ...]]: + """Yield one tuple of values per core row, holding `terms` in the order given. + + A faster alternative to iterating over the reader when only a few terms are needed. + See :meth:`dwca.files.CSVDataFile.iter_terms`. + + Usage:: + + for identifier, latitude, longitude in dwca.iter_terms( + [qn('occurrenceID'), qn('decimalLatitude'), qn('decimalLongitude')]): + pass + + The special name "id" requests the core file's id column - the same name used by + :attr:`dwca.descriptors.DataFileDescriptor.headers`. It resolves even when the + Metafile declares no ```` for that column, which is the common case. If the + core file declares an actual term named "id" (possible in metafile-less archives, + where terms are raw CSV header names), that declared term takes precedence, which + matches what `CoreRow.data['id']` already returns. + + :param terms: a list of full term identifiers. + :raises ValueError: if any of `terms` is not present in the core data file. + """ + return self.core_file.iter_terms(terms) + def orphaned_extension_rows(self) -> Dict[str, Dict[str, List[int]]]: """Return a dict of the orphaned extension rows. @@ -296,10 +325,12 @@ def use_extensions(self) -> bool: return (self.descriptor is not None) and (len(self.descriptor.extensions) > 0) @property - # TODO: decide, test and document what we guarantee about ordering def rows(self) -> List[CoreRow]: """A list of :class:`rows.CoreRow` objects representing the content of the archive. + The list is in order of appearance in the core data file, the same order produced by + iterating the reader. + .. warning:: All rows will be loaded in memory. In case of a large Darwin Core Archive, you may prefer using a for loop. @@ -341,8 +372,8 @@ def get_corerow_by_position(self, position: int) -> CoreRow: .. note:: - - If index is bigger than the length of the archive, None is returned - - The position is often an appropriate way to unambiguously identify a core row in a DwCA. + The position is often an appropriate way to unambiguously identify a core row in + a DwCA. """ for i, row in enumerate(self): @@ -536,22 +567,39 @@ def core_contains_term(self, term_url: str) -> bool: """Return `True` if the Core file of the archive contains the `term_url` term.""" return term_url in self.core_file.file_descriptor.terms - def __iter__(self) -> "DwCAReader": - self._corefile_pointer = 0 - return self + def __iter__(self) -> Iterator[CoreRow]: + # A fresh iterator each time, so nesting loops (or calling get_corerow_by_id() from + # inside one) behaves as expected. + return self._iter_core_rows() + + def _iter_core_rows(self) -> Iterator[CoreRow]: + extension_files = self.extension_files + source_metadata = self.source_metadata + + for row in self.core_file.iter_rows(): + # Set up linked data so the CoreRow will know about them + row.link_extension_files(extension_files) + row.link_source_metadata(source_metadata) + yield row def __next__(self): return self.next() def next(self) -> CoreRow: # NOQA - try: - row = self.core_file.get_row_by_position(self._corefile_pointer) - - # Set up linked data so the CoreRow will know about them - row.link_extension_files(self.extension_files) - row.link_source_metadata(self.source_metadata) + """Return the next core row. + + .. deprecated:: + Iterate over the reader instead. This method keeps its own implicit iterator, + which is independent of any `for row in reader:` loop: interleaving the two makes + each of them scan the archive separately. Once this iterator is exhausted it + raises StopIteration, and the call after that starts a fresh pass from the first + row rather than raising again. + """ + if self._default_iterator is None: + self._default_iterator = self._iter_core_rows() - self._corefile_pointer = self._corefile_pointer + 1 - return row - except IndexError: - raise StopIteration + try: + return next(self._default_iterator) + except StopIteration: + self._default_iterator = None + raise diff --git a/dwca/rows.py b/dwca/rows.py index 5b7a4b0..09ecdf2 100644 --- a/dwca/rows.py +++ b/dwca/rows.py @@ -1,11 +1,9 @@ """Objects that represents data rows coming from DarwinCore Archives.""" import csv -import sys -from typing import Dict, Optional +from typing import Dict, List, Optional from dwca.descriptors import DataFileDescriptor -from dwca.exceptions import InvalidArchive class Row(object): @@ -14,6 +12,8 @@ class Row(object): This class is intended to be subclassed rather than used directly. """ + __slots__ = ("descriptor", "position", "rowtype", "raw_fields", "data") + # Common ground for __str__ between subclasses def _build_str(self, source_str, id_str): txt = ( @@ -52,33 +52,50 @@ def _build_str(self, source_str, id_str): def __init__( self, csv_line: str, position: int, datafile_descriptor: DataFileDescriptor ) -> None: - #: An instance of :class:`dwca.descriptors.DataFileDescriptor` describing the originating - #: data file. + self._populate( + csv_line_to_fields( + csv_line, + line_ending=datafile_descriptor.lines_terminated_by, + field_ending=datafile_descriptor.fields_terminated_by, + fields_enclosed_by=datafile_descriptor.fields_enclosed_by, + ), + position, + datafile_descriptor, + ) + + @classmethod + def from_fields( + cls, + raw_fields: List[str], + position: int, + datafile_descriptor: DataFileDescriptor, + ) -> "Row": + """Build a Row from an already-split data row. + + This is the constructor used by the streaming engine. :meth:`__init__`, which takes a + raw CSV line, is kept for backwards compatibility. + """ + row = cls.__new__(cls) + row._populate(raw_fields, position, datafile_descriptor) + return row + + def _populate(self, raw_fields, position, datafile_descriptor) -> None: + #: An instance of :class:`dwca.descriptors.DataFileDescriptor` describing the + #: originating data file. self.descriptor = datafile_descriptor # type: DataFileDescriptor - #: The row position/index (starting at 0) in the source data file. This can be used, for example with - #: :meth:`dwca.read.DwCAReader.get_corerow_by_position` or :meth:`dwca.files.CSVDataFile.get_row_by_position`. + #: The row position/index (starting at 0) in the source data file. This can be used, + #: for example with :meth:`dwca.read.DwCAReader.get_corerow_by_position` or + #: :meth:`dwca.files.CSVDataFile.get_row_by_position`. self.position = position # type: int - #: The csv line type as stated in the archive descriptor. - #: (or None if the archive has no descriptor). Examples: - #: http://rs.tdwg.org/dwc/terms/Occurrence, + #: The csv line type as stated in the archive descriptor (or None if the archive has + #: no descriptor). Examples: http://rs.tdwg.org/dwc/terms/Occurrence, #: http://rs.gbif.org/terms/1.0/VernacularName, ... self.rowtype = self.descriptor.type # type: Optional[str] - # self.raw_fields is a list of the csv_line's content - #: - self.raw_fields = csv_line_to_fields( - csv_line, - line_ending=self.descriptor.lines_terminated_by, - field_ending=self.descriptor.fields_terminated_by, - fields_enclosed_by=self.descriptor.fields_enclosed_by, - ) - - # TODO: raw_fields is a new property: to test - - # TODO: Consistency check ?? self.raw_fields length should be : - # num of self.raw_fields described in core_meta + 2 (id and \n) + #: A list of the row's raw (unmapped) field values. + self.raw_fields = raw_fields #: A dict containing the Row data, such as:: #: @@ -90,29 +107,11 @@ def __init__( #: #: myrow.data['http://rs.tdwg.org/dwc/terms/locality'] # => "Brussels" #: - #: .. note:: The :func:`dwca.darwincore.utils.qualname` helper is available to make such calls less verbose. - self.data = {} # type: Dict[str, str] - - for field_descriptor in self.descriptor.fields: - try: - column_index = int(field_descriptor["index"]) - field_row_value = self.raw_fields[column_index] - except TypeError: - # int() argument must be a string... We don't have an index for this field - field_row_value = None - except IndexError: - msg = ( - "The descriptor references a non-existent field (index={i})".format( - i=column_index - ) - ) - raise InvalidArchive(msg) - - field_default_value = field_descriptor["default"] - - self.data[field_descriptor["term"]] = ( - field_row_value or field_default_value or "" - ) + #: .. note:: The :func:`dwca.darwincore.utils.qualname` helper is available to make + #: such calls less verbose. + self.data = datafile_descriptor.field_plan.build_data( + raw_fields + ) # type: Dict[str, str] class CoreRow(Row): @@ -127,14 +126,14 @@ def __str__(self) -> str: id_str = "Row id: " + str(self.id) return super(CoreRow, self)._build_str("Core file", id_str) - def __init__( - self, csv_line: str, position: int, datafile_descriptor: DataFileDescriptor - ) -> None: - super(CoreRow, self).__init__(csv_line, position, datafile_descriptor) + __slots__ = ("id", "source_metadata", "extension_data_files", "_extensions") - if self.descriptor.id_index is not None: + def _populate(self, raw_fields, position, datafile_descriptor) -> None: + super(CoreRow, self)._populate(raw_fields, position, datafile_descriptor) + + if datafile_descriptor.id_index is not None: #: The row id - self.id = self.raw_fields[self.descriptor.id_index] + self.id = raw_fields[datafile_descriptor.id_index] else: self.id = None @@ -183,25 +182,42 @@ def extensions(self): # Should these 3 be factorized ? How ? Mixin ? Parent class ? def __key(self): """Return a tuple representing the row. Common ground between equality and hash.""" + # A CoreRow obtained without going through DwCAReader iteration (e.g. + # CSVDataFile.get_row_by_position() called directly) never had link_extension_files() + # / link_source_metadata() called on it, so self.extensions and self.source_metadata + # don't exist. Fall back to None for those rows instead of letting the attribute + # access raise AttributeError. This doesn't change equality for linked rows (both + # attributes are always present after DwCAReader.next() links them). + extensions = self.extensions if hasattr(self, "extension_data_files") else None + source_metadata = ( + self.source_metadata if hasattr(self, "source_metadata") else None + ) + return ( self.descriptor, self.id, self.data, - self.extensions, - self.source_metadata, + extensions, + source_metadata, self.rowtype, self.raw_fields, self.position, ) def __eq__(self, other): - return self.__key() == other.__key() + # The isinstance test is required, not just defensive: __key is name-mangled, so + # other.__key() means other._CoreRow__key(), which an ExtensionRow (or any non-row) + # doesn't have. Without it, comparing to anything else raises AttributeError. + if not isinstance(other, CoreRow): + return NotImplemented - def __ne__(self, other): - return not self.__eq__(other) + return self.__key() == other.__key() def __hash__(self): - return hash(self.__key()) + # __key() embeds the data dict, the raw field list and the lazily-loaded extensions, + # none of which are hashable. Equal rows still hash equally because this is a subset + # of the equality key. + return hash((self.descriptor, self.id, self.rowtype, self.position)) class ExtensionRow(Row): @@ -215,13 +231,13 @@ def __str__(self): id_str = "Core row id: " + str(self.core_id) return super(ExtensionRow, self)._build_str("Extension file", id_str) - def __init__( - self, csv_line: str, position: int, datafile_descriptor: DataFileDescriptor - ) -> None: - super(ExtensionRow, self).__init__(csv_line, position, datafile_descriptor) + __slots__ = ("core_id",) + + def _populate(self, raw_fields, position, datafile_descriptor) -> None: + super(ExtensionRow, self)._populate(raw_fields, position, datafile_descriptor) #: The id of the core row this extension row is referring to. - self.core_id = self.raw_fields[datafile_descriptor.coreid_index] + self.core_id = raw_fields[datafile_descriptor.coreid_index] def __key(self): """Return a tuple representing the row. Common ground between equality and hash.""" @@ -235,13 +251,15 @@ def __key(self): ) def __eq__(self, other): - return self.__key() == other.__key() + # See the note on CoreRow.__eq__: __key is name-mangled, so this test is what keeps + # comparisons against a CoreRow (or anything else) from raising AttributeError. + if not isinstance(other, ExtensionRow): + return NotImplemented - def __ne__(self, other): - return not self.__eq__(other) + return self.__key() == other.__key() def __hash__(self): - return hash(self.__key()) + return hash((self.descriptor, self.core_id, self.rowtype, self.position)) def csv_line_to_fields(csv_line, line_ending, field_ending, fields_enclosed_by): @@ -249,16 +267,25 @@ def csv_line_to_fields(csv_line, line_ending, field_ending, fields_enclosed_by): Return a list of fields. Content is not trimmed. """ - csv_line = csv_line.rstrip(line_ending) - raw_fields = [] + # A carriage return is stripped alongside the declared terminator: archives routinely + # declare "\n" while the file itself has CRLF line endings. This matches both the csv + # module's own behavior and CSVDataFile._iter_field_lists, so the two access paths agree. + csv_line = csv_line.rstrip(line_ending + "\r") if fields_enclosed_by == "": - opts = {"quoting": csv.QUOTE_NONE} - else: - opts = {"quoting": csv.QUOTE_ALL, "quotechar": fields_enclosed_by} - - for row in csv.reader([csv_line], delimiter=field_ending, **opts): - for f in row: - field = f.strip(fields_enclosed_by) - raw_fields.append(field) - return raw_fields + # No enclosure: the line is simply split on the separator. This also keeps any + # quote character that happens to appear in the content. + return csv_line.split(field_ending) + + # The csv module unwraps the enclosure and un-doubles escaped quote characters itself. + # Stripping the quote character afterwards would eat a legitimate leading or trailing + # one from the field's own content. + for row in csv.reader( + [csv_line], + delimiter=field_ending, + quotechar=fields_enclosed_by, + quoting=csv.QUOTE_MINIMAL, + ): + return row + + return [] diff --git a/dwca/star_record.py b/dwca/star_record.py index 1e71757..a3ff49d 100644 --- a/dwca/star_record.py +++ b/dwca/star_record.py @@ -1,6 +1,5 @@ from dwca.files import CSVDataFile -from typing import List -from typing_extensions import Literal +from typing import List, Literal import itertools diff --git a/dwca/test/archive_builder.py b/dwca/test/archive_builder.py new file mode 100644 index 0000000..ac90927 --- /dev/null +++ b/dwca/test/archive_builder.py @@ -0,0 +1,188 @@ +"""Build synthetic Darwin Core Archives for tests. + +Produces a directory-based archive (DwCAReader reads directories directly, so nothing +needs zipping). Every parameter that the Metafile can express is explicit, which is what +lets the characterization tests cover encodings, terminators, quoting and header counts +without committing more binary sample files. +""" + +import os +import shutil +import tempfile +from xml.sax.saxutils import quoteattr + +TERM_PREFIX = "http://rs.tdwg.org/dwc/terms/" + + +def temp_archive_dir(test_case): + """Return a temporary directory that is removed when `test_case` finishes. + + Several tests in the suite count the entries in the system temporary directory, so + leaking one per test would eventually make them flaky. + """ + directory = tempfile.mkdtemp() + test_case.addCleanup(shutil.rmtree, directory, ignore_errors=True) + return directory + +METAFILE_TEMPLATE = """ + + occurrence.txt + +{fields} + +{extension} + +""" + +EXTENSION_TEMPLATE = """ + extension.txt + +{fields} + """ + + +def _escape(raw): + # The Metafile stores separators escaped, e.g. the tab character as the two + # characters backslash-t. Mirror what real archives contain. + escaped = ( + raw.replace("\\", "\\\\") + .replace("\t", "\\t") + .replace("\n", "\\n") + .replace("\r", "\\r") + ) + return quoteattr(escaped) + + +def _field_tags(terms, defaults, indent=" "): + lines = [] + for index, term in enumerate(terms): + default = (defaults or {}).get(term) + if default is None: + lines.append( + '{i}'.format(i=indent, n=index, t=term) + ) + else: + lines.append( + '{i}'.format( + i=indent, n=index, t=term, d=quoteattr(default) + ) + ) + for term, default in (defaults or {}).items(): + if term not in terms: + lines.append( + '{i}'.format( + i=indent, t=term, d=quoteattr(default) + ) + ) + return "\n".join(lines) + + +def _render_rows(rows, fields_terminated_by, lines_terminated_by, fields_enclosed_by): + out = [] + for row in rows: + if fields_enclosed_by: + cells = [ + fields_enclosed_by + + cell.replace(fields_enclosed_by, fields_enclosed_by * 2) + + fields_enclosed_by + for cell in row + ] + else: + cells = list(row) + out.append(fields_terminated_by.join(cells)) + return lines_terminated_by.join(out) + (lines_terminated_by if out else "") + + +def build_archive( + directory, + rows, + columns=None, + encoding="utf-8", + lines_terminated_by="\n", + fields_terminated_by="\t", + fields_enclosed_by="", + ignore_header_lines=0, + header_rows=(), + id_index=0, + terms=None, + defaults=None, + extension=None, + trailing_newline=True, + raw_payload=None, +): + """Write a Darwin Core Archive into `directory` and return its path. + + :param rows: list of lists of str, the data rows. + :param terms: list of full term URIs, one per column. Defaults to term0, term1, ... + :param defaults: dict term -> default value. A term not present in `terms` becomes a + default-only field (no index attribute), as the standard allows. + :param extension: list of rows for an extension data file, or None. Column 0 is the coreid. + :param raw_payload: bytes written verbatim as the core data file instead of `rows`. + Used to build inputs no well-formed writer would produce (undecodable bytes, ragged + rows, blank lines). + """ + if columns is None: + columns = max([len(r) for r in rows] + [1]) if rows else 1 + if terms is None: + terms = [TERM_PREFIX + "term" + str(i) for i in range(columns)] + + metafile = METAFILE_TEMPLATE.format( + encoding=quoteattr(encoding), + fields_terminated_by=_escape(fields_terminated_by), + lines_terminated_by=_escape(lines_terminated_by), + fields_enclosed_by=_escape(fields_enclosed_by), + ignore_header_lines=quoteattr(str(ignore_header_lines)), + id_index=id_index, + fields=_field_tags(terms, defaults), + extension=( + "" + if extension is None + else EXTENSION_TEMPLATE.format( + encoding=quoteattr(encoding), + fields_terminated_by=_escape(fields_terminated_by), + lines_terminated_by=_escape(lines_terminated_by), + fields_enclosed_by=_escape(fields_enclosed_by), + ignore_header_lines=quoteattr(str(ignore_header_lines)), + fields=_field_tags( + [TERM_PREFIX + "vernacularName"], None, indent=" " + ).replace('index="0"', 'index="1"'), + ) + ), + ) + + with open(os.path.join(directory, "meta.xml"), "w", encoding="utf-8") as f: + f.write(metafile) + + data_path = os.path.join(directory, "occurrence.txt") + if raw_payload is not None: + with open(data_path, "wb") as f: + f.write(raw_payload) + else: + payload = _render_rows( + list(header_rows) + list(rows), + fields_terminated_by, + lines_terminated_by, + fields_enclosed_by, + ) + if not trailing_newline and payload.endswith(lines_terminated_by): + payload = payload[: -len(lines_terminated_by)] + with open(data_path, "w", encoding=encoding, newline="") as f: + f.write(payload) + + if extension is not None: + payload = _render_rows( + list(header_rows) + list(extension), + fields_terminated_by, + lines_terminated_by, + fields_enclosed_by, + ) + with open( + os.path.join(directory, "extension.txt"), "w", encoding=encoding, newline="" + ) as f: + f.write(payload) + + return directory diff --git a/dwca/test/test_characterization.py b/dwca/test/test_characterization.py new file mode 100644 index 0000000..4235a34 --- /dev/null +++ b/dwca/test/test_characterization.py @@ -0,0 +1,617 @@ +"""Characterization tests: these pin CURRENT behavior so a rewrite of the parsing +engine produces a visible diff rather than a silent change. + +Tests marked with a `# CHARACTERIZATION: wrong, see B` comment assert behavior we +know to be incorrect. Phase 1 changes them deliberately. +""" + +import unittest +from array import array + +import pytest + +from dwca.exceptions import InvalidArchive +from dwca.read import DwCAReader + +from .archive_builder import build_archive, temp_archive_dir +from .helpers import sample_data_path + +TERM1 = "http://rs.tdwg.org/dwc/terms/term1" +VERNACULAR_TERM = "http://rs.tdwg.org/dwc/terms/vernacularName" + + +class TestArchiveBuilder(unittest.TestCase): + def test_builds_a_readable_archive(self): + path = build_archive( + temp_archive_dir(self), rows=[["1", "Borneo"], ["2", "Mumbai"]] + ) + + with DwCAReader(path) as dwca: + rows = list(dwca) + + assert 2 == len(rows) + assert "1" == rows[0].id + assert "Borneo" == rows[0].data[TERM1] + assert "Mumbai" == rows[1].data[TERM1] + + def test_raw_payload_is_written_verbatim(self): + path = build_archive( + temp_archive_dir(self), rows=[], columns=2, raw_payload=b"1\tBorneo\n" + ) + + with DwCAReader(path) as dwca: + assert ["Borneo"] == [row.data[TERM1] for row in dwca] + + +class TestHeaderLines(unittest.TestCase): + """ignoreHeaderLines, for BOTH access paths. + + The two paths used to disagree: DwCAReader iteration went through get_row_by_position() + (which offset correctly), while CSVDataFile.__iter__ used readlines(hint), where the + argument is a byte-size hint rather than a line count. coreid_index was built from the + second path, so it used to pick up leftover header lines. Both paths now agree. + """ + + def _archive(self, ignore_header_lines, header_rows): + return build_archive( + temp_archive_dir(self), + rows=[["1", "Borneo"], ["2", "Mumbai"]], + ignore_header_lines=ignore_header_lines, + header_rows=header_rows, + ) + + def test_no_header(self): + path = self._archive(0, []) + + with DwCAReader(path) as dwca: + assert ["1", "2"] == [row.id for row in dwca] + assert "1" == dwca.core_file.get_row_by_position(0).id + + def test_one_header(self): + path = self._archive(1, [["id", "locality"]]) + + with DwCAReader(path) as dwca: + assert ["1", "2"] == [row.id for row in dwca] + assert "1" == dwca.core_file.get_row_by_position(0).id + assert {"1": [0], "2": [1]} == { + k: list(v) for k, v in dwca.core_file.coreid_index.items() + } + + def test_two_headers_iteration_and_random_access_agree(self): + path = self._archive(2, [["idA", "locA"], ["idB", "locB"]]) + + with DwCAReader(path) as dwca: + assert ["1", "2"] == [row.id for row in dwca] + assert "1" == dwca.core_file.get_row_by_position(0).id + + assert {"1": [0], "2": [1]} == { + k: list(v) for k, v in dwca.core_file.coreid_index.items() + } + + +class TestLineTerminators(unittest.TestCase): + def test_unix_terminator(self): + path = build_archive( + temp_archive_dir(self), + rows=[["1", "Borneo"], ["2", "Mumbai"]], + lines_terminated_by="\n", + ) + + with DwCAReader(path) as dwca: + assert ["Borneo", "Mumbai"] == [row.data[TERM1] for row in dwca] + + def test_dos_terminator(self): + path = build_archive( + temp_archive_dir(self), + rows=[["1", "Borneo"], ["2", "Mumbai"]], + lines_terminated_by="\r\n", + ) + + with DwCAReader(path) as dwca: + assert ["Borneo", "Mumbai"] == [row.data[TERM1] for row in dwca] + + def test_crlf_file_declaring_a_bare_lf_terminator(self): + """A CRLF file whose Metafile declares "\\n" must not leave a stray CR on the row. + + This combination is common rather than exotic: git checks text files out with CRLF on + Windows, so an archive committed to a repository hits it without anyone choosing it. + The library's own dwca-simple-dir fixture behaves this way on a Windows runner, which + is how a regression here was caught. + """ + path = build_archive( + temp_archive_dir(self), + rows=[], + columns=2, + raw_payload=b"1\tBorneo\r\n2\tMumbai\r\n", + ) + + with DwCAReader(path) as dwca: + streamed = [row.data[TERM1] for row in dwca] + seeked = [ + dwca.core_file.get_row_by_position(i).data[TERM1] for i in range(2) + ] + + assert ["Borneo", "Mumbai"] == streamed + assert streamed == seeked + + def test_multichar_terminator_is_rejected_by_python_io(self): + path = build_archive( + temp_archive_dir(self), + rows=[["1", "Borneo"]], + lines_terminated_by="@@\n", + ) + + # io.open() only accepts None, '', '\n', '\r' and '\r\n' as newline, so an archive + # declaring anything else cannot be opened at all. Pinned so a rewrite that moves + # off the newline= parameter has to decide what to do about it. + with pytest.raises(ValueError): + DwCAReader(path) + + def test_unicode_next_line_does_not_split_a_row(self): + """U+0085 must not be treated as a line break (issue #20). + + str.splitlines() splits on U+0085 but io line iteration does not, so any rewrite + using splitlines() would silently double the row count here. + """ + path = build_archive( + temp_archive_dir(self), + rows=[], + columns=2, + raw_payload=b"1\tbefore\xc2\x85after\n", # U+0085 encoded as UTF-8 + ) + + with DwCAReader(path) as dwca: + rows = list(dwca) + + assert 1 == len(rows) + assert "before\u0085after" == rows[0].data[TERM1] + + +class TestEncodings(unittest.TestCase): + def test_windows1252_data_file(self): + path = build_archive( + temp_archive_dir(self), + rows=[["1", "caf\xe9"], ["2", "na\xefve"]], + encoding="windows-1252", + ) + + with DwCAReader(path) as dwca: + assert ["caf\xe9", "na\xefve"] == [row.data[TERM1] for row in dwca] + + def test_latin1_data_file(self): + path = build_archive( + temp_archive_dir(self), rows=[["1", "caf\xe9"]], encoding="latin-1" + ) + + with DwCAReader(path) as dwca: + assert ["caf\xe9"] == [row.data[TERM1] for row in dwca] + + def test_utf8_bom_leaks_into_the_first_field(self): + path = build_archive( + temp_archive_dir(self), + rows=[], + columns=2, + raw_payload=b"\xef\xbb\xbf1\tBorneo\n", # UTF-8 byte order mark + ) + + with DwCAReader(path) as dwca: + rows = list(dwca) + + # Surprising but not a known bug: the encoding is "utf-8", not "utf-8-sig", so the + # byte order mark becomes part of the id instead of being stripped. A rewrite adding + # BOM handling would change this deliberately. + assert "\ufeff1" == rows[0].id + + def test_undecodable_byte_does_not_break_random_access(self): + path = build_archive( + temp_archive_dir(self), + rows=[], + columns=2, + raw_payload=b"1\tcaf\xe9\n2\tMumbai\n3\tBorneo\n", + ) + + with DwCAReader(path) as dwca: + # Row 0 is fine: the offset index has not drifted yet. + assert "caf\ufffd" == dwca.core_file.get_row_by_position(0).data[TERM1] + + assert "Mumbai" == dwca.core_file.get_row_by_position(1).data[ + "http://rs.tdwg.org/dwc/terms/term1" + ] + + assert ["caf\ufffd", "Mumbai", "Borneo"] == [ + row.data["http://rs.tdwg.org/dwc/terms/term1"] for row in dwca + ] + + +class TestQuoting(unittest.TestCase): + def _read_localities(self, payload): + path = build_archive( + temp_archive_dir(self), + rows=[], + columns=2, + fields_terminated_by=",", + fields_enclosed_by='"', + raw_payload=payload, + ) + + with DwCAReader(path) as dwca: + return [row.data[TERM1] for row in dwca] + + def test_delimiter_inside_a_quoted_field(self): + """Regression guard for the v0.11.0 fix. Any hand-rolled parser breaks this.""" + assert ["plain, with comma"] == self._read_localities( + b'"1","plain, with comma"\n' + ) + + def test_quote_in_the_middle_of_content(self): + assert ['say "hi" there'] == self._read_localities(b'"1","say ""hi"" there"\n') + + def test_quote_at_the_edge_of_content_is_preserved(self): + assert ['say "hi"'] == self._read_localities(b'"1","say ""hi"""\n') + assert ['"hi" she said'] == self._read_localities(b'"1","""hi"" she said"\n') + + def test_quote_characters_are_kept_when_the_archive_declares_no_enclosure(self): + path = build_archive( + temp_archive_dir(self), + rows=[], + columns=2, + raw_payload=b'1\t"betta" splendens\n', + ) + + with DwCAReader(path) as dwca: + assert '"betta" splendens' == list(dwca)[0].data[TERM1] + + +class TestDegenerateRows(unittest.TestCase): + def test_row_with_fewer_columns_than_declared(self): + path = build_archive( + temp_archive_dir(self), rows=[], columns=3, raw_payload=b"1\tBorneo\n" + ) + + with DwCAReader(path) as dwca: + with pytest.raises(InvalidArchive): + list(dwca) + + def test_row_with_more_columns_than_declared(self): + path = build_archive( + temp_archive_dir(self), + rows=[], + columns=2, + raw_payload=b"1\tBorneo\textra\n", + ) + + with DwCAReader(path) as dwca: + rows = list(dwca) + + # Extra columns are ignored by data but kept in raw_fields. + assert "Borneo" == rows[0].data[TERM1] + assert ["1", "Borneo", "extra"] == rows[0].raw_fields + + def test_blank_line_in_the_middle(self): + path = build_archive( + temp_archive_dir(self), + rows=[], + columns=2, + raw_payload=b"1\tBorneo\n\n2\tMumbai\n", + ) + + with DwCAReader(path) as dwca: + with pytest.raises(InvalidArchive): + list(dwca) + + def test_no_trailing_newline_at_eof(self): + path = build_archive( + temp_archive_dir(self), + rows=[], + columns=2, + raw_payload=b"1\tBorneo\n2\tMumbai", + ) + + with DwCAReader(path) as dwca: + assert ["Borneo", "Mumbai"] == [row.data[TERM1] for row in dwca] + + def test_empty_core_file(self): + path = build_archive( + temp_archive_dir(self), rows=[], columns=2, raw_payload=b"" + ) + + with DwCAReader(path) as dwca: + assert [] == list(dwca) + assert [] == dwca.rows + + def test_header_only_core_file(self): + path = build_archive( + temp_archive_dir(self), + rows=[], + columns=2, + ignore_header_lines=1, + raw_payload=b"id\tlocality\n", + ) + + with DwCAReader(path) as dwca: + assert [] == list(dwca) + + +class TestIterationSemantics(unittest.TestCase): + def _archive(self): + return build_archive( + temp_archive_dir(self), + rows=[["1", "Borneo"], ["2", "Mumbai"], ["3", "Paris"]], + ) + + def test_sequential_re_iteration(self): + with DwCAReader(self._archive()) as dwca: + assert 3 == len(list(dwca)) + assert 3 == len(list(dwca)) + + def test_nested_iteration(self): + with DwCAReader(self._archive()) as dwca: + pairs = [(outer.id, inner.id) for outer in dwca for inner in dwca] + + assert 9 == len(pairs) + + def test_lookup_inside_a_loop(self): + seen = [] + with DwCAReader(self._archive()) as dwca: + for row in dwca: + seen.append(row.id) + assert "2" == dwca.get_corerow_by_id("2").id + + assert ["1", "2", "3"] == seen + + def test_random_access_interleaved_with_iteration_is_safe(self): + with DwCAReader(self._archive()) as dwca: + seen = [] + for row in dwca: + seen.append(row.id) + # This path is offset-based, so it does not disturb iteration. + assert "1" == dwca.core_file.get_row_by_position(0).id + + assert ["1", "2", "3"] == seen + + +class TestExtensionFiles(unittest.TestCase): + """extension= was never passed to build_archive by any test, so that whole branch of the + builder was dead code, and every extension-bearing behavior below ran only on the three + bundled sample archives, which all happen to share one configuration (utf-8, tab, no + enclosure, ignoreHeaderLines="1"). CSVDataFile.coreid_index used to be built through + CSVDataFile.__iter__ (the readlines(byte-hint) header bug, see TestHeaderLines above and + B1), and get_all_rows_by_coreid() fed those positions into get_row_by_position(), which + re-applied lines_to_ignore - exactly the seam the B1 bug lived in, and it was unpinned for + extensions until this test was added. coreid_index is now built through iter_rows() + instead, so that seam no longer exists here. + """ + + def _archive(self, **kwargs): + return build_archive( + temp_archive_dir(self), + rows=[["1", "Borneo"], ["2", "Mumbai"]], + extension=[["1", "elephant"], ["1", "tiger"], ["2", "monkey"]], + **kwargs, + ) + + def test_coreid_index(self): + path = self._archive() + + with DwCAReader(path) as dwca: + index = dwca.extension_files[0].coreid_index + + assert {"1": array("L", [0, 1]), "2": array("L", [2])} == index + + def test_get_all_rows_by_coreid(self): + path = self._archive() + + with DwCAReader(path) as dwca: + ext = dwca.extension_files[0] + + several = ext.get_all_rows_by_coreid("1") + assert ["elephant", "tiger"] == [r.data[VERNACULAR_TERM] for r in several] + + one = ext.get_all_rows_by_coreid("2") + assert ["monkey"] == [r.data[VERNACULAR_TERM] for r in one] + + assert [] == ext.get_all_rows_by_coreid("unknown") + + def test_orphaned_extension_rows(self): + path = build_archive( + temp_archive_dir(self), + rows=[["1", "Borneo"], ["2", "Mumbai"]], + extension=[["1", "elephant"], ["99", "ghost"]], + ) + + with DwCAReader(path) as dwca: + assert {"extension.txt": {"99": [1]}} == dwca.orphaned_extension_rows() + + def test_core_row_extensions_content_and_ordering(self): + path = self._archive() + + with DwCAReader(path) as dwca: + rows = list(dwca) + + assert ["elephant", "tiger"] == [ + r.data[VERNACULAR_TERM] for r in rows[0].extensions + ] + assert ["monkey"] == [r.data[VERNACULAR_TERM] for r in rows[1].extensions] + + def test_ignore_header_lines_are_skipped_in_the_extension_index(self): + path = self._archive( + ignore_header_lines=2, header_rows=[["idA", "locA"], ["idB", "locB"]] + ) + + with DwCAReader(path) as dwca: + index = dwca.extension_files[0].coreid_index + + assert {"1": array("L", [0, 1]), "2": array("L", [2])} == index + + +class TestRandomAccessDirect(unittest.TestCase): + """DwCAReader.next() is implemented as CSVDataFile.get_row_by_position() (see + dwca/read.py), so today every iteration test incidentally exercises random access too. + A rewrite that stops delegating would make that coverage disappear silently unless + something calls get_row_by_position() directly, which is what these tests do. + """ + + def test_multibyte_utf8(self): + # "\u4e2d\u6587" ("Chinese" in Chinese) is 2 characters but 6 bytes in UTF-8, so the + # byte offset of row 1 (used internally for seek()) differs from what a + # character-based offset would be. + locality = "caf\u00e9 \u4e2d\u6587" + path = build_archive( + temp_archive_dir(self), + rows=[["1", locality], ["2", "Mumbai"]], + ) + + with DwCAReader(path) as dwca: + first = dwca.core_file.get_row_by_position(0) + second = dwca.core_file.get_row_by_position(1) + + assert locality == first.data[TERM1] + assert ["1", locality] == first.raw_fields + assert "Mumbai" == second.data[TERM1] + assert ["2", "Mumbai"] == second.raw_fields + + def test_dos_terminator(self): + path = build_archive( + temp_archive_dir(self), + rows=[["1", "Borneo"], ["2", "Mumbai"]], + lines_terminated_by="\r\n", + ) + + with DwCAReader(path) as dwca: + first = dwca.core_file.get_row_by_position(0) + second = dwca.core_file.get_row_by_position(1) + + assert "Borneo" == first.data[TERM1] + assert ["1", "Borneo"] == first.raw_fields + assert "Mumbai" == second.data[TERM1] + assert ["2", "Mumbai"] == second.raw_fields + + def test_quoted_field_containing_the_separator(self): + path = build_archive( + temp_archive_dir(self), + rows=[["1", "plain, with comma"], ["2", "second"]], + fields_terminated_by=",", + fields_enclosed_by='"', + ) + + with DwCAReader(path) as dwca: + first = dwca.core_file.get_row_by_position(0) + second = dwca.core_file.get_row_by_position(1) + + assert "plain, with comma" == first.data[TERM1] + assert ["1", "plain, with comma"] == first.raw_fields + assert "second" == second.data[TERM1] + assert ["2", "second"] == second.raw_fields + + +class TestRowExtensionsDuringCoreIteration(unittest.TestCase): + def _archive(self): + return build_archive( + temp_archive_dir(self), + rows=[["1", "Borneo"], ["2", "Mumbai"]], + extension=[["1", "elephant"], ["1", "tiger"], ["2", "monkey"]], + ) + + def test_row_extensions_accessible_during_core_iteration(self): + path = self._archive() + + seen = {} + with DwCAReader(path) as dwca: + for row in dwca: + seen[row.id] = [e.data[VERNACULAR_TERM] for e in row.extensions] + + assert {"1": ["elephant", "tiger"], "2": ["monkey"]} == seen + + def test_row_extensions_are_cached(self): + path = self._archive() + + with DwCAReader(path) as dwca: + row = next(iter(dwca)) + first = row.extensions + second = row.extensions + + # Lazy-loaded and cached on the instance: same list object both times, not rebuilt. + assert first is second + assert ["elephant", "tiger"] == [e.data[VERNACULAR_TERM] for e in first] + + +class TestNegativePosition(unittest.TestCase): + def test_negative_position_returns_the_header_line(self): + # get_row_by_position() computes self._line_offsets[position + lines_to_ignore]. For + # position=-1 this wraps around to the last entry in the offsets array, which is the + # header line (kept in the index but skipped during normal iteration). + with DwCAReader(sample_data_path("dwca-simple-test-archive.zip")) as dwca: + row = dwca.core_file.get_row_by_position(-1) + + # CHARACTERIZATION: wrong, see B7. A rewrite will likely raise IndexError instead. + assert "id" == row.id + + +class TestDuplicateCoreIds(unittest.TestCase): + """get_corerow_by_id()'s docstring disclaims which row wins when ids repeat, and + coreid_index maps one id to several positions. Pin both so a rewrite has to decide + deliberately rather than by accident. + """ + + def _archive(self): + return build_archive( + temp_archive_dir(self), + rows=[["1", "Borneo"], ["1", "Mumbai"], ["2", "Paris"]], + ) + + def test_get_corerow_by_id_returns_the_first_match(self): + with DwCAReader(self._archive()) as dwca: + row = dwca.get_corerow_by_id("1") + + assert "Borneo" == row.data[TERM1] + + def test_coreid_index_holds_every_position(self): + with DwCAReader(self._archive()) as dwca: + index = dwca.core_file.coreid_index + + assert {"1": array("L", [0, 1]), "2": array("L", [2])} == index + + +class TestQuotedRecordsSpanningLines(unittest.TestCase): + """A quoted field may contain the line terminator. Iteration and random access must agree.""" + + QUOTED_META = {"fields_terminated_by": ",", "fields_enclosed_by": '"'} + + def _archive(self, payload, columns=3): + return build_archive( + temp_archive_dir(self), + rows=[], + columns=columns, + raw_payload=payload, + **self.QUOTED_META + ) + + def test_iteration_and_random_access_agree(self): + path = self._archive(b'0,b,a\n1,a,"x\ny\nz"\n2,"x\ny\nz",a\n') + + with DwCAReader(path) as dwca: + streamed = [row.raw_fields for row in dwca] + seeked = [ + dwca.core_file.get_row_by_position(i).raw_fields + for i in range(len(streamed)) + ] + + assert [["0", "b", "a"], ["1", "a", "x\ny\nz"], ["2", "x\ny\nz", "a"]] == streamed + assert streamed == seeked + + def test_extensions_are_not_truncated(self): + """coreid_index is built from the streaming pass but consumed through seeks.""" + directory = temp_archive_dir(self) + path = build_archive( + directory, + rows=[["1", "Lagopus"], ["2", "Struthio"]], + fields_terminated_by=",", + fields_enclosed_by='"', + extension=[["1", "grouse\nfoo"], ["2", "ostrich"]], + ) + + with DwCAReader(path) as dwca: + per_core = [[e.raw_fields for e in row.extensions] for row in dwca] + + assert [["1", "grouse\nfoo"]] == per_core[0] + assert [["2", "ostrich"]] == per_core[1] diff --git a/dwca/test/test_datafile.py b/dwca/test/test_datafile.py index 5fb9a3d..966db5d 100644 --- a/dwca/test/test_datafile.py +++ b/dwca/test/test_datafile.py @@ -1,3 +1,4 @@ +import os import unittest import xml.etree.ElementTree as ET from array import array @@ -35,6 +36,9 @@ def test_coreid_index(self): description_txt = extension_files[0] vernacular_txt = extension_files[1] + # coreid_index values are array("L") rather than lists. This is documented in the + # property docstring and DwCAReader.orphaned_extension_rows() relies on it via + # .tolist(), so it is part of the contract, not an implementation detail. expected_core = { "1": array("L", [0]), "2": array("L", [1]), @@ -49,9 +53,6 @@ def test_coreid_index(self): expected_description = {"1": array("L", [0, 1]), "4": array("L", [2])} assert description_txt.coreid_index == expected_description - with pytest.raises(AttributeError): - dwca.corefile.coreid_index - def test_file_descriptor_attribute(self): """The instance of DataFileDescriptor passed to the constructor is available in .file_descriptor""" @@ -160,3 +161,285 @@ def test_iterate(self): for row in data_file: assert isinstance(row, str) + + +class TestStreamingIteration(unittest.TestCase): + def test_iter_rows_yields_every_row_in_order(self): + with DwCAReader(sample_data_path("dwca-ids.zip")) as dwca: + rows = list(dwca.core_file.iter_rows()) + + # Row IDs appear in the core file in this order: 4-1-3-2 + assert ["4", "1", "3", "2"] == [row.id for row in rows] + assert [0, 1, 2, 3] == [row.position for row in rows] + + def test_iter_rows_agrees_with_random_access(self): + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + for data_file in [dwca.core_file] + dwca.extension_files: + streamed = list(data_file.iter_rows()) + seeked = [ + data_file.get_row_by_position(i) for i in range(len(streamed)) + ] + + assert [r.data for r in streamed] == [r.data for r in seeked] + assert [r.raw_fields for r in streamed] == [ + r.raw_fields for r in seeked + ] + + def test_iter_rows_can_be_nested(self): + """Each call gets its own stream, so concurrent passes do not interfere.""" + with DwCAReader(sample_data_path("dwca-ids.zip")) as dwca: + pairs = [ + (outer.id, inner.id) + for outer in dwca.core_file.iter_rows() + for inner in dwca.core_file.iter_rows() + ] + + assert 16 == len(pairs) + + def test_iter_rows_on_a_quoted_archive(self): + with DwCAReader(sample_data_path("dwca-csv-quote-dir")) as dwca: + rows = list(dwca.core_file.iter_rows()) + + assert 2 == len(rows) + + def test_raw_line_iteration_skips_every_header_line(self): + """readlines() takes a byte-size hint, not a line count, so with two header lines + the second used to leak through as data.""" + from .archive_builder import build_archive, temp_archive_dir + + path = build_archive( + temp_archive_dir(self), + rows=[["1", "Borneo"], ["2", "Mumbai"]], + ignore_header_lines=2, + header_rows=[["idA", "locA"], ["idB", "locB"]], + ) + with DwCAReader(path) as dwca: + lines = list(dwca.core_file) + + assert 2 == len(lines) + assert lines[0].startswith("1\t") + assert lines[1].startswith("2\t") + + +class TestLineOffsets(unittest.TestCase): + def test_opening_an_archive_does_not_build_the_index(self): + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + assert dwca.core_file._line_offsets is None + + dwca.core_file.get_row_by_position(0) + + assert dwca.core_file._line_offsets is not None + + def test_offsets_survive_an_undecodable_byte(self): + from .archive_builder import build_archive, temp_archive_dir + + path = build_archive( + temp_archive_dir(self), + rows=[], + columns=2, + raw_payload=b"1\tcaf\xe9\n2\tMumbai\n3\tBorneo\n", + ) + with DwCAReader(path) as dwca: + term = "http://rs.tdwg.org/dwc/terms/term1" + + assert "Mumbai" == dwca.core_file.get_row_by_position(1).data[term] + assert "Borneo" == dwca.core_file.get_row_by_position(2).data[term] + + def test_offsets_with_dos_line_endings(self): + from .archive_builder import build_archive, temp_archive_dir + + path = build_archive( + temp_archive_dir(self), + rows=[["1", "Borneo"], ["2", "Mumbai"]], + lines_terminated_by="\r\n", + ) + with DwCAReader(path) as dwca: + term = "http://rs.tdwg.org/dwc/terms/term1" + + assert "Borneo" == dwca.core_file.get_row_by_position(0).data[term] + assert "Mumbai" == dwca.core_file.get_row_by_position(1).data[term] + + def test_offsets_are_correct_across_a_chunk_boundary(self): + """The scanner reads in chunks, so a terminator can straddle two reads.""" + from dwca.files import _build_line_offsets + from .archive_builder import build_archive, temp_archive_dir + + rows = [[str(i), "locality-" + str(i)] for i in range(5000)] + path = build_archive(temp_archive_dir(self), rows=rows) + + data_path = os.path.join(path, "occurrence.txt") + reference = _build_line_offsets(data_path, "utf-8", "\n") + chunked = _build_line_offsets(data_path, "utf-8", "\n", chunk_size=7) + + assert 5000 == len(reference) + assert list(reference) == list(chunked) + + def test_a_zero_byte_core_file_has_no_rows(self): + """A zero-byte file must report no lines at all, not one spurious empty line.""" + from .archive_builder import build_archive, temp_archive_dir + + path = build_archive( + temp_archive_dir(self), rows=[], columns=2, raw_payload=b"" + ) + with DwCAReader(path) as dwca: + with pytest.raises(IndexError): + dwca.core_file.get_row_by_position(0) + + def test_multibyte_utf8_and_an_embedded_newline_in_the_same_file(self): + """_read_record() must use a byte-accurate read. + + offsets are byte offsets, but a text stream's read(n) counts characters, so a + naive `self._file_stream.read(next_offset - start)` would desync as soon as the + file contains a character that takes more than one byte in UTF-8 - truncating or + over-reading the record. This archive puts multi-byte characters (which make + byte count and character count diverge) and a quoted embedded newline (which + makes a record span more than one physical line) in the same file, so both + failure modes would have to hold simultaneously to pass. + """ + from .archive_builder import build_archive, temp_archive_dir + + # ACCENTED_E is a 2-byte UTF-8 character, SNOWMAN a 3-byte one: both make byte + # count and character count diverge. Row 2's third field is quoted and contains + # an embedded newline, so that record spans two physical lines. + ACCENTED_E = "\xe9" + SNOWMAN = "\u2603" + payload = ( + "1,caf" + ACCENTED_E + "," + SNOWMAN + "\n" + '2,"multi\nline",' + SNOWMAN + ACCENTED_E + "\n" + "3," + ACCENTED_E * 3 + ",end\n" + ).encode("utf-8") + path = build_archive( + temp_archive_dir(self), + rows=[], + columns=3, + raw_payload=payload, + fields_terminated_by=",", + fields_enclosed_by='"', + ) + + with DwCAReader(path) as dwca: + streamed = [row.raw_fields for row in dwca] + seeked = [ + dwca.core_file.get_row_by_position(i).raw_fields + for i in range(len(streamed)) + ] + + assert [ + ["1", "caf" + ACCENTED_E, SNOWMAN], + ["2", "multi\nline", SNOWMAN + ACCENTED_E], + ["3", ACCENTED_E * 3, "end"], + ] == streamed + assert streamed == seeked + + +class TestIterTerms(unittest.TestCase): + def test_matches_the_row_api(self): + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + terms = sorted(dwca.core_file.file_descriptor.terms) + + via_rows = [tuple(row.data[t] for t in terms) for row in dwca] + via_terms = list(dwca.core_file.iter_terms(terms)) + + assert via_rows == via_terms + + def test_works_on_an_extension_file(self): + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + extension = dwca.extension_files[0] + terms = sorted(extension.file_descriptor.terms) + + via_rows = [tuple(r.data[t] for t in terms) for r in extension.iter_rows()] + via_terms = list(extension.iter_terms(terms)) + + assert via_rows == via_terms + + def test_subset_of_columns(self): + with DwCAReader(sample_data_path("dwca-simple-test-archive.zip")) as dwca: + values = list( + dwca.core_file.iter_terms(["http://rs.tdwg.org/dwc/terms/locality"]) + ) + + assert [("Borneo",), ("Mumbai",)] == values + + def test_unknown_term_raises_before_reading_anything(self): + with DwCAReader(sample_data_path("dwca-simple-test-archive.zip")) as dwca: + with pytest.raises(ValueError): + dwca.core_file.iter_terms(["http://rs.tdwg.org/dwc/terms/nope"]) + + def test_default_only_term(self): + with DwCAReader(sample_data_path("dwca-test-default.zip")) as dwca: + values = list( + dwca.core_file.iter_terms(["http://rs.tdwg.org/dwc/terms/country"]) + ) + + assert [("Belgium",), ("Belgium",)] == values + + def test_can_be_nested(self): + with DwCAReader(sample_data_path("dwca-ids.zip")) as dwca: + term = ["http://rs.tdwg.org/dwc/terms/family"] + pairs = [ + (a, b) + for a in dwca.core_file.iter_terms(term) + for b in dwca.core_file.iter_terms(term) + ] + + assert 16 == len(pairs) + + +class TestIterTermsKeyColumns(unittest.TestCase): + def test_core_id_is_reachable(self): + """dwca-ids.zip declares no for its id column.""" + with DwCAReader(sample_data_path("dwca-ids.zip")) as dwca: + from_rows = [row.id for row in dwca] + from_terms = [values[0] for values in dwca.iter_terms(["id"])] + + assert ["4", "1", "3", "2"] == from_rows + assert from_rows == from_terms + + def test_extension_coreid_is_reachable(self): + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + extension = dwca.extension_files[0] + + from_rows = [row.core_id for row in extension.iter_rows()] + from_terms = [values[0] for values in extension.iter_terms(["coreid"])] + + assert from_rows == from_terms + + def test_key_column_mixes_with_real_terms_in_the_requested_order(self): + order = "http://rs.tdwg.org/dwc/terms/order" + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + got = list(dwca.iter_terms([order, "id"])) + expected = [(row.data[order], row.id) for row in dwca] + + assert expected == got + + def test_id_is_still_unknown_when_the_file_has_no_id_column(self): + """A metafile-less archive has no id column, so the name must not resolve.""" + with DwCAReader(sample_data_path("dwca-simple-csv.zip")) as dwca: + with pytest.raises(ValueError): + dwca.iter_terms(["id"]) + + def test_coreid_is_unknown_on_a_core_file(self): + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + with pytest.raises(ValueError): + dwca.iter_terms(["coreid"]) + + +class TestClosedFileGuarantee(unittest.TestCase): + def test_iteration_after_close_raises(self): + """close() documents that content is not accessible in any way afterwards.""" + with DwCAReader(sample_data_path("dwca-simple-dir")) as dwca: + data_file = dwca.core_file + + assert list(data_file.iter_rows()) # works while open + data_file.close() + + with pytest.raises(ValueError): + list(data_file.iter_rows()) + + def test_reader_iteration_after_close_raises(self): + dwca = DwCAReader(sample_data_path("dwca-simple-dir")) + assert list(dwca) + dwca.close() + + with pytest.raises(ValueError): + list(dwca) diff --git a/dwca/test/test_descriptors.py b/dwca/test/test_descriptors.py index a16e702..faf5f3b 100644 --- a/dwca/test/test_descriptors.py +++ b/dwca/test/test_descriptors.py @@ -3,8 +3,11 @@ import xml.etree.ElementTree as ET import zipfile +import pytest + from dwca.darwincore.utils import qualname as qn from dwca.descriptors import DataFileDescriptor, ArchiveDescriptor +from dwca.exceptions import InvalidArchive from dwca.read import DwCAReader from .helpers import sample_data_path @@ -455,6 +458,102 @@ def test_exposes_core_terms(self): assert fields == descriptor.core.terms +class TestDataFileDescriptorEquality(unittest.TestCase): + """Unit tests for DataFileDescriptor equality and hashing. + + Descriptors compare by value (the data file layout they describe) rather than by object + identity, so descriptors built twice from the same archive - by two DwCAReader instances, + for example - compare equal. + """ + + CORE_SECTION = """ + + + occurrence.txt + + + + + + """ + + def _make(self, section=None): + return DataFileDescriptor.make_from_metafile_section( + ET.fromstring(section if section is not None else self.CORE_SECTION) + ) + + def test_descriptors_from_identical_sections_are_equal(self): + one = self._make() + two = self._make() + + assert one is not two + # raw_element is an ET.Element, which compares by identity: these two are distinct + # objects, so equality can only hold if raw_element is left out of the comparison. + assert one.raw_element is not two.raw_element + assert one == two + assert not (one != two) + assert hash(one) == hash(two) + assert len({one, two}) == 1 + + def test_descriptors_of_the_same_archive_read_twice_are_equal(self): + path = sample_data_path("dwca-2extensions.zip") + + with DwCAReader(path) as one, DwCAReader(path) as two: + assert one.descriptor.core == two.descriptor.core + + for ext_one, ext_two in zip( + one.descriptor.extensions, two.descriptor.extensions + ): + assert ext_one == ext_two + + def test_core_and_extension_descriptors_differ(self): + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + core = dwca.descriptor.core + + for extension in dwca.descriptor.extensions: + assert core != extension + + def test_descriptors_differing_by_ignored_header_lines_differ(self): + one = self._make() + two = self._make( + self.CORE_SECTION.replace('ignoreHeaderLines="0"', 'ignoreHeaderLines="1"') + ) + + # Everything but the number of header lines to skip is identical here. That number + # lives in raw_element, which is excluded from the comparison, so it's only taken into + # account through the lines_to_ignore property. + assert one.lines_to_ignore != two.lines_to_ignore + assert one != two + + def test_descriptors_differing_by_fields_differ(self): + one = self._make() + two = self._make( + self.CORE_SECTION.replace( + 'term="http://rs.tdwg.org/dwc/terms/scientificName"', + 'term="http://rs.tdwg.org/dwc/terms/locality"', + ) + ) + + assert one != two + + def test_descriptors_differing_by_file_location_differ(self): + one = self._make() + two = self._make( + self.CORE_SECTION.replace("occurrence.txt", "other_occurrences.txt") + ) + + assert one != two + + def test_comparison_with_other_types_returns_false(self): + descriptor = self._make() + + assert descriptor != "not a descriptor" + assert descriptor != 42 + assert descriptor != None # noqa: E711 - we're testing __eq__, not identity + assert not (descriptor == "not a descriptor") + + class TestArchiveDescriptor(unittest.TestCase): """Unit tests for ArchiveDescriptor class.""" @@ -594,3 +693,235 @@ def test_exposes_metadata_filename(self): descriptor = dwca.descriptor assert descriptor.metadata_filename == "eml.xml" + + +class TestHeadersIndexZero(unittest.TestCase): + def test_column_at_index_zero_is_not_dropped(self): + """A metafile-less archive has no id_index, so column 0 comes only from fields.""" + with DwCAReader(sample_data_path("dwca-simple-csv.zip")) as dwca: + descriptor = dwca.core_file.file_descriptor + + assert len(descriptor.fields) == len(descriptor.headers) + assert "gbifid" == descriptor.headers[0] + + +class TestFieldPlan(unittest.TestCase): + def _descriptor(self, fields_xml, tag="core", id_tag=''): + section = """ + <{tag} encoding="utf-8" fieldsTerminatedBy="\\t" linesTerminatedBy="\\n" \ +fieldsEnclosedBy="" ignoreHeaderLines="0" rowType="http://rs.tdwg.org/dwc/terms/Occurrence"> + occurrence.txt + {id_tag} + {fields} + + """.format( + tag=tag, id_tag=id_tag, fields=fields_xml + ) + return DataFileDescriptor.make_from_metafile_section(ET.fromstring(section)) + + def test_contiguous_columns(self): + descriptor = self._descriptor( + '' + '' + ) + + assert {"http://x/a": "1", "http://x/b": "Borneo"} == ( + descriptor.field_plan.build_data(["1", "Borneo"]) + ) + + def test_columns_out_of_order_and_with_gaps(self): + descriptor = self._descriptor( + '' + '' + ) + + assert {"http://x/a": "third", "http://x/b": "first"} == ( + descriptor.field_plan.build_data(["first", "second", "third"]) + ) + + def test_single_column(self): + descriptor = self._descriptor('') + + assert {"http://x/a": "Borneo"} == descriptor.field_plan.build_data( + ["1", "Borneo"] + ) + + def test_default_only_field_has_no_column(self): + descriptor = self._descriptor( + '' + '' + ) + + assert {"http://x/a": "1", "http://x/country": "Belgium"} == ( + descriptor.field_plan.build_data(["1"]) + ) + + def test_default_fills_an_empty_cell(self): + """A field can have both a column and a default (issue #80).""" + descriptor = self._descriptor( + '' + '' + ) + + assert "Borneo" == descriptor.field_plan.build_data(["1", "Borneo"])["http://x/b"] + assert "fallback" == descriptor.field_plan.build_data(["1", ""])["http://x/b"] + + def test_missing_value_without_default_becomes_empty_string(self): + descriptor = self._descriptor( + '' + '' + ) + + assert "" == descriptor.field_plan.build_data(["1", ""])["http://x/b"] + + def test_key_order_follows_the_metafile(self): + """Row.data key order is visible through str(row), so it must not drift.""" + descriptor = self._descriptor( + '' + '' + '' + ) + + assert ["http://x/a", "http://x/country", "http://x/b"] == list( + descriptor.field_plan.build_data(["1", "Borneo"]) + ) + + def test_row_with_too_few_columns_raises(self): + descriptor = self._descriptor('') + + with pytest.raises(InvalidArchive): + descriptor.field_plan.build_data(["1", "Borneo"]) + + def test_the_plan_is_cached(self): + descriptor = self._descriptor('') + + assert descriptor.field_plan is descriptor.field_plan + + +class TestTermGetter(unittest.TestCase): + def _plan(self, fields_xml): + section = """ + + occurrence.txt + + {fields} + + """.format( + fields=fields_xml + ) + descriptor = DataFileDescriptor.make_from_metafile_section( + ET.fromstring(section) + ) + return descriptor.field_plan + + def test_returns_values_in_the_requested_order(self): + plan = self._plan( + '' + '' + '' + ) + getter = plan.term_getter(["http://x/c", "http://x/a"]) + + assert ("third", "first") == getter(["first", "second", "third"]) + + def test_a_single_term_still_yields_a_tuple(self): + plan = self._plan('') + getter = plan.term_getter(["http://x/b"]) + + assert ("second",) == getter(["first", "second"]) + + def test_no_terms(self): + plan = self._plan('') + getter = plan.term_getter([]) + + assert () == getter(["first"]) + + def test_a_term_may_be_requested_twice(self): + plan = self._plan('') + getter = plan.term_getter(["http://x/a", "http://x/a"]) + + assert ("first", "first") == getter(["first"]) + + def test_default_only_term_yields_the_constant(self): + plan = self._plan( + '' + '' + ) + getter = plan.term_getter(["http://x/country", "http://x/a"]) + + assert ("Belgium", "first") == getter(["first"]) + + def test_default_fills_an_empty_cell(self): + plan = self._plan( + '' + '' + ) + getter = plan.term_getter(["http://x/b"]) + + assert ("value",) == getter(["first", "value"]) + assert ("fallback",) == getter(["first", ""]) + + def test_missing_value_without_default_becomes_empty_string(self): + plan = self._plan( + '' + '' + ) + getter = plan.term_getter(["http://x/b"]) + + assert ("",) == getter(["first", ""]) + + def test_unknown_terms_are_named_in_the_error(self): + plan = self._plan('') + + with pytest.raises(ValueError) as excinfo: + plan.term_getter(["http://x/missing", "http://x/a", "http://x/gone"]) + + message = str(excinfo.value) + assert "http://x/missing" in message + assert "http://x/gone" in message + assert "http://x/a" not in message + + def test_short_row_raises_invalid_archive(self): + plan = self._plan( + '' + '' + ) + getter = plan.term_getter(["http://x/e"]) + + with pytest.raises(InvalidArchive): + getter(["first", "second"]) + + def test_terms_may_be_a_one_shot_iterable(self): + # term_getter() used to iterate `terms` three times internally (missing, indexes, + # defaults). A generator is exhausted after the first pass, which silently made + # every later pass - and therefore every returned tuple - empty. + plan = self._plan( + '' + '' + ) + expected = ("second", "first") + + terms_list = ["http://x/b", "http://x/a"] + assert expected == plan.term_getter(t for t in terms_list)(["first", "second"]) + + # A tuple and a set of a single term must still work (the normalisation must not + # break non-list iterables that already worked before). + assert expected == plan.term_getter(tuple(terms_list))(["first", "second"]) + assert ("second",) == plan.term_getter({"http://x/b"})(["first", "second"]) + + def test_a_declared_term_named_id_wins_over_the_key_column(self): + """Terms in metafile-less archives are raw header names, so one can be "id". + + The declared term wins, which is what row.data["id"] already returns. + """ + section = """ + + occurrence.txt + + + + """ + descriptor = DataFileDescriptor.make_from_metafile_section(ET.fromstring(section)) + getter = descriptor.field_plan.term_getter(["id"]) + + assert ("declared",) == getter(["key", "declared"]) diff --git a/dwca/test/test_dwcareader.py b/dwca/test/test_dwcareader.py index 8484670..74f8ce9 100644 --- a/dwca/test/test_dwcareader.py +++ b/dwca/test/test_dwcareader.py @@ -3,7 +3,6 @@ import unittest import xml.etree.ElementTree as ET -import pandas as pd from unittest.mock import patch from dwca.darwincore.utils import qualname as qn @@ -12,9 +11,18 @@ from dwca.files import CSVDataFile from dwca.read import DwCAReader from dwca.rows import CoreRow, ExtensionRow +from dwca.vendor import _has_pandas from .helpers import sample_data_path import pytest +# Pandas is an optional dependency of the library, so the test suite has to run without +# it too. The tests that genuinely exercise pd_read() are skipped when it is absent; the +# one that checks pd_read's behavior WITHOUT pandas deliberately still runs. +if _has_pandas: + import pandas as pd + +requires_pandas = unittest.skipUnless(_has_pandas, "pandas is not installed") + class TestPandasIntegration(unittest.TestCase): """Tests of Pandas integration features.""" @@ -31,6 +39,7 @@ def test_pd_read_pandas_unavailable(self): with pytest.raises(ImportError): dwca.pd_read("occurrence.txt") + @requires_pandas def test_pd_read_simple_case(self): with DwCAReader(sample_data_path("dwca-simple-test-archive.zip")) as dwca: df = dwca.pd_read("occurrence.txt") @@ -56,6 +65,7 @@ def test_pd_read_simple_case(self): "betta splendens", ] + @requires_pandas def test_pd_read_chunked_default_value(self): """Pandas chuncksize should not be used with default values. @@ -66,6 +76,7 @@ def test_pd_read_chunked_default_value(self): for chunk in dwca.pd_read("occurrence.txt", chunksize=1): pass + @requires_pandas def test_pd_read_chunked(self): """If no default values are available in the archive, chunksize should work. @@ -75,6 +86,7 @@ def test_pd_read_chunked(self): for chunk in dwca.pd_read("occurrence.txt", chunksize=2): assert isinstance(chunk, pd.DataFrame) + @requires_pandas def test_pd_read_no_data_files(self): with DwCAReader(sample_data_path("dwca-simple-test-archive.zip")) as dwca: with pytest.raises(NotADataFile): @@ -83,6 +95,7 @@ def test_pd_read_no_data_files(self): with pytest.raises(NotADataFile): dwca.pd_read("eml.xml") + @requires_pandas def test_pd_read_extensions(self): with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: desc_df = dwca.pd_read("description.txt") @@ -95,6 +108,7 @@ def test_pd_read_extensions(self): assert vern_df.shape == (4, 4) assert vern_df["countryCode"].values.tolist() == ["US", "ZA", "FI", "ZA"] + @requires_pandas def test_pd_read_quotedir(self): with DwCAReader(sample_data_path("dwca-csv-quote-dir")) as dwca: df = dwca.pd_read("occurrence.txt") @@ -102,6 +116,7 @@ def test_pd_read_quotedir(self): assert df.shape == (2, 5) assert df["basisOfRecord"].values.tolist()[0] == "Observation, something" + @requires_pandas def test_pd_read_default_values(self): with DwCAReader(sample_data_path("dwca-test-default.zip")) as dwca: df = dwca.pd_read("occurrence.txt") @@ -110,6 +125,7 @@ def test_pd_read_default_values(self): for country in df["country"].values.tolist(): assert country == "Belgium" + @requires_pandas def test_pd_read_utf8_eol_ignored(self): """Ensure we don't split lines based on the x85 utf8 EOL char. @@ -121,13 +137,20 @@ def test_pd_read_utf8_eol_ignored(self): # (61 - and probably an IndexError - if errors) assert 64 == df.shape[1] + @requires_pandas def test_pd_read_simple_csv(self): with DwCAReader(sample_data_path("dwca-simple-csv.zip")) as dwca: df = dwca.pd_read("0008333-160118175350007.csv") - # Ensure we get the correct number of rows + # Ensure we get the correct number of rows and columns assert 3 == df.shape[0] - # Ensure we can access arbitrary data + assert 42 == df.shape[1] + # This archive has no metafile, so the first column (gbifid) has no id_index to + # rely on. It must come from the headers list as a regular column, not be silently + # promoted to the DataFrame index by pandas because of a missing header name. + assert "gbifid" in df.columns + assert df.index.name is None + # Ensure we can access arbitrary data assert df["decimallatitude"].values.tolist()[1] == -31.98333 @@ -257,11 +280,15 @@ def test_descriptor_references_non_existent_data_field(self): pass def test_custom_tempdir(self): - tmp_dir = os.path.abspath(".tmp") - with DwCAReader( - sample_data_path("dwca-simple-test-archive.zip"), tmp_dir=tmp_dir - ) as dwca: - assert dwca.absolute_temporary_path("occurrence.txt").startswith(tmp_dir) + previous_tempdir = tempfile.tempdir + try: + tmp_dir = os.path.abspath(".tmp") + with DwCAReader( + sample_data_path("dwca-simple-test-archive.zip"), tmp_dir=tmp_dir + ) as dwca: + assert dwca.absolute_temporary_path("occurrence.txt").startswith(tmp_dir) + finally: + tempfile.tempdir = previous_tempdir def test_use_extensions(self): """Ensure the .use_extensions attribute of DwCAReader works as intended.""" @@ -533,10 +560,9 @@ def test_row_human_representation(self): assert "Row id:" in l_repr assert "Reference extension rows: No" in l_repr assert "Reference source metadata: No" in l_repr - assert ( - "http://rs.tdwg.org/dwc/terms/scientificName': 'tetraodon fluviatilis'" - in l_repr - ) + # Assert the value reaches the representation, not how Python formats a dict. + assert "tetraodon fluviatilis" in l_repr + assert "tetraodon fluviatilis" == l.data[qn("scientificName")] with DwCAReader(sample_data_path("dwca-star-test-archive.zip")) as star_dwca: l = star_dwca.rows[0] @@ -1068,6 +1094,42 @@ def test_whitespace_before_xml_tag(self): # The next line will throw an exception if metadata.xml can't be parsed DwCAReader(sample_data_path("gbif-results-whitespace-in-xml.zip")) + def test_nested_iteration_is_independent(self): + with DwCAReader(sample_data_path("dwca-ids.zip")) as dwca: + pairs = [(outer.id, inner.id) for outer in dwca for inner in dwca] + + assert 16 == len(pairs) + + def test_lookup_inside_a_loop_terminates(self): + with DwCAReader(sample_data_path("dwca-ids.zip")) as dwca: + seen = [] + for row in dwca: + seen.append(row.id) + dwca.get_corerow_by_id("1") + + assert ["4", "1", "3", "2"] == seen + + def test_next_still_works(self): + dwca = DwCAReader(sample_data_path("dwca-ids.zip")) + try: + assert "4" == dwca.next().id + assert "1" == dwca.next().id + finally: + dwca.close() + + def test_iter_terms_on_the_reader(self): + with DwCAReader(sample_data_path("dwca-simple-test-archive.zip")) as dwca: + values = list( + dwca.iter_terms( + [ + "http://rs.tdwg.org/dwc/terms/locality", + "http://rs.tdwg.org/dwc/terms/family", + ] + ) + ) + + assert [("Borneo", "Tetraodontidae"), ("Mumbai", "Osphronemidae")] == values + if __name__ == "__main__": unittest.main() diff --git a/dwca/test/test_rows.py b/dwca/test/test_rows.py index 543ff61..492fb69 100644 --- a/dwca/test/test_rows.py +++ b/dwca/test/test_rows.py @@ -47,3 +47,176 @@ def test_position(self): assert 0 == vernacular_first_line.position assert 1 == vernacular_second_line.position assert 2 == vernacular_third_line.position + + +class TestRowHashing(unittest.TestCase): + def test_core_rows_are_hashable(self): + """CHANGES.txt has claimed rows are hashable since 0.3.3, but __key() embedded a + dict and a list, so hash() raised TypeError for every row.""" + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + rows = dwca.rows + + assert len(set(rows)) == len(rows) + + def test_extension_rows_are_hashable(self): + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + extension_rows = dwca.rows[0].extensions + + assert len(set(extension_rows)) == len(extension_rows) + + def test_unlinked_core_rows_are_comparable(self): + """A CoreRow obtained directly from CSVDataFile.get_row_by_position() (rather than by + iterating the DwCAReader) is never linked to extension files or source metadata, so + self.extension_data_files and self.source_metadata don't exist on it. __key() used to + access self.extensions and self.source_metadata unconditionally, so hash()/__eq__ on + such a row raised AttributeError - which set() and put in a set both trigger.""" + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + one = dwca.core_file.get_row_by_position(0) + two = dwca.core_file.get_row_by_position(0) + + assert one is not two + assert one == two + assert hash(one) == hash(two) + assert len({one, two}) == 1 + + def test_equal_rows_hash_equally(self): + # Two distinct objects that compare equal must hash equal. + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + one = dwca.rows[0] + two = dwca.rows[0] + + assert one is not two + assert one == two + assert hash(one) == hash(two) + + +class TestRowEquality(unittest.TestCase): + """Rows compare by value, including across readers. + + Row equality used to embed the DataFileDescriptor, which compared by object identity: rows + read from two DwCAReader instances over the same archive never compared equal, even with + identical data, raw_fields, id, position and rowtype. + """ + + def test_core_rows_from_two_readers_over_the_same_archive_are_equal(self): + path = sample_data_path("dwca-2extensions.zip") + + with DwCAReader(path) as one, DwCAReader(path) as two: + row_one = one.rows[0] + row_two = two.rows[0] + + assert row_one.descriptor is not row_two.descriptor + assert row_one == row_two + assert not (row_one != row_two) + assert hash(row_one) == hash(row_two) + assert len({row_one, row_two}) == 1 + + def test_extension_rows_from_two_readers_over_the_same_archive_are_equal(self): + path = sample_data_path("dwca-2extensions.zip") + + with DwCAReader(path) as one, DwCAReader(path) as two: + row_one = one.rows[0].extensions[0] + row_two = two.rows[0].extensions[0] + + assert row_one.descriptor is not row_two.descriptor + assert row_one == row_two + assert hash(row_one) == hash(row_two) + assert len({row_one, row_two}) == 1 + + def test_different_core_rows_are_not_equal(self): + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + assert dwca.rows[0] != dwca.rows[1] + + def test_comparing_a_core_row_to_an_extension_row_returns_false(self): + """__key() is name-mangled, so CoreRow.__eq__ used to call other._CoreRow__key() on an + ExtensionRow, which doesn't have it: the comparison raised AttributeError instead of + returning False.""" + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + core_row = dwca.rows[0] + extension_row = core_row.extensions[0] + + assert core_row != extension_row + assert extension_row != core_row + assert not (core_row == extension_row) + + def test_comparing_rows_with_other_types_returns_false(self): + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + core_row = dwca.rows[0] + extension_row = core_row.extensions[0] + + for row in (core_row, extension_row): + assert row != "not a row" + assert row != 42 + assert row != None # noqa: E711 - we're testing __eq__, not identity + assert not (row == "not a row") + + def test_rows_of_different_types_can_share_a_set(self): + """A CoreRow and an ExtensionRow landing in the same hash bucket must compare, not + raise.""" + with DwCAReader(sample_data_path("dwca-2extensions.zip")) as dwca: + core_row = dwca.rows[0] + extension_row = core_row.extensions[0] + + assert len({core_row, extension_row}) == 2 + + +class TestRowFromFields(unittest.TestCase): + def _descriptor(self): + import xml.etree.ElementTree as ET + + from dwca.descriptors import DataFileDescriptor + + section = """ + + occurrence.txt + + + + + """ + return DataFileDescriptor.make_from_metafile_section(ET.fromstring(section)) + + def test_from_fields_matches_the_raw_line_constructor(self): + from dwca.rows import CoreRow + + descriptor = self._descriptor() + + from_line = CoreRow("1\tBorneo\n", 0, descriptor) + from_fields = CoreRow.from_fields(["1", "Borneo"], 0, descriptor) + + assert from_line.data == from_fields.data + assert from_line.raw_fields == from_fields.raw_fields + assert from_line.id == from_fields.id + assert from_line.position == from_fields.position + assert from_line.rowtype == from_fields.rowtype + + def test_from_fields_returns_the_right_class(self): + from dwca.rows import CoreRow + + row = CoreRow.from_fields(["1", "Borneo"], 0, self._descriptor()) + + assert isinstance(row, CoreRow) + + +class TestCsvLineToFieldsQuoting(unittest.TestCase): + def test_quote_at_the_edge_of_content_is_preserved(self): + """The csv module already un-doubles escaped quotes; stripping afterwards ate a + legitimate leading or trailing quote from the field's own content.""" + assert ['1', 'say "hi"'] == csv_line_to_fields( + '"1","say ""hi"""', "\n", ",", '"' + ) + assert ['1', '"hi" she said'] == csv_line_to_fields( + '"1","""hi"" she said"', "\n", ",", '"' + ) + + def test_delimiter_inside_a_quoted_field_still_works(self): + """Regression guard for the v0.11.0 fix.""" + assert ["field 1", "field 2, with comma", "field 3"] == csv_line_to_fields( + 'field 1,"field 2, with comma",field 3', "\n", ",", '"' + ) + + def test_unenclosed_line_keeps_quote_characters(self): + assert ["1", '"betta" splendens'] == csv_line_to_fields( + '1\t"betta" splendens', "\n", "\t", "" + ) diff --git a/requirements-dev.txt b/requirements-dev.txt index d07437e..ff7f233 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,4 +1,3 @@ pandas mock==2.0.0 pytest -typing-extensions \ No newline at end of file diff --git a/setup.py b/setup.py index 3b1cb32..c44a4b5 100644 --- a/setup.py +++ b/setup.py @@ -12,13 +12,15 @@ license="BSD licence, see LICENCE.txt", description="A simple Python package to read Darwin Core Archive (DwC-A) files.", long_description=open("README.rst").read(), + python_requires=">=3.8", classifiers=[ "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: 3.13", "Programming Language :: Python :: Implementation :: PyPy", ], )