Conversation
… missing paths Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rop Itemization `Row` is mapping-first: by name the value is typed (float / datetime.date / raw str / None per Q6), by position it is the raw field. Column names and per-column kinds come from `fec_parser::mappings` and are shared, interned, per (row_type, fec_version) through a process-wide schema cache. Adds slices, `keys`/`values`/`items`/`get`, `__contains__`, `extra_fields`, `__eq__`/`__hash__`, pickling via `_row_from_parts`, and `line`. `Mapping.register(Row)` so `pd.DataFrame(rows)` yields one column per name. `csv::StringRecord::from_byte_record_lossy` drops the record position when the line is not valid UTF-8, so `FilingRow` now carries `line`, read off the `ByteRecord` before the lossy conversion. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…prefix filter Replaces the eager native `Filing` (which drained every row into a `Vec` under the GIL, then cloned on every getter) with `libfec_parser.open(src)`, returning a single-pass `FilingReader`: - `__next__` pulls 256 rows at a time inside `Python::detach`; rejected rows never become Python objects. - `rows(*prefixes)` filters by row-type prefix in Rust, case-insensitively. - context manager, `close()`/`closed`, `id`, `fec_version`, `source_length`. - `header`/`cover`/`cover_row` are parsed eagerly and identity-stable `Py<T>`s; `Header`/`Cover` are now `frozen`, and the cover dates are `datetime.date`. - an unmapped row type raises `MissingMappingError(row_type, version, line)` and leaves the reader usable; a CSV error closes it and raises `FecParseError`. `Filing` is gone until ticket 16 re-creates it in pure Python, so `test_parser.py::TestFiling` is `xfail(strict=True)` for one commit; the tests that only used `Filing` to get at rows were ported to `open()`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…tions deprecated Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The test did a fixed 50 parses of the 263 KB fixture and then asserted the window was at least 50 ms wide before counting the ticker thread's ticks. On a `--release` extension — what CI installs, and what `make notebook-check` leaves behind — 50 parses take under 30 ms, so that precondition tripped. Parse in a loop until ~0.25 s of wall time has passed instead, which makes the window the same size under both profiles, and drop the now-redundant elapsed assertion. The tick threshold stays deliberately loose. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…r text-mode files One resolver, `src/source.rs::resolve`, now decides what `open()`, `read()`, `Filing()` and `fec_header()` accept, in this order: 1. the buffer protocol (`bytes`, `bytearray`, `memoryview`, `mmap`) — read in place through a `PyBuffer<u8>`, no copy; a non-contiguous buffer (a strided `memoryview`) is gathered into a `Vec` first; 2. `str`/`os.PathLike` — a path, opened as a `File`; 3. a text-mode file (`io.TextIOBase`) — `TypeError` naming `'rb'`; 4. anything with `read` — pulled 64 KiB at a time, never `.read()` whole; 5. anything else — `TypeError`. The buffer branch has to precede the path branch: `PathBuf` extraction goes through `os.fspath`, which accepts `bytes` as a path. Exceptions raised by a source's own `read()` come back as themselves: the parser only ever hands back its own error types, which this crate wraps as `FecParseError` by `Display`, so the adapter stashes the `PyErr` in a slot the reader prefers over the wrapped error. `id`/`source_length` now come from a file object too (`.name`'s stem, `os.fstat(fileno()).st_size`), `0`/`None` when it cannot be known. Measured on the 91 MB benchmark filing, iterating a `bytes` source to the end peaks at 124 MB RSS against a 91 MB bytes object — 33 MB of overhead, of which 25 MB is the interpreter, the import and the read itself. A file object peaks at 32 MB total. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Pulling a row from a binary file object runs Python code (`read()`) from inside `refill`, which holds `inner` — so a thread can hold one of the reader's mutexes while waiting to attach to the interpreter. A second thread holding the GIL and blocking on the same mutex closed the cycle: A waited for the GIL, B held the GIL and waited for the mutex, and the whole interpreter wedged. Before ticket 15 this was safe, because a pull never needed the GIL. Two rules, now documented on `FilingReader`: 1. Every lock taken with the GIL held goes through `lock_attached`, which is `MutexExt::lock_py_attached` (try once, else detach, block, re-attach) plus the same poison recovery as `lock`. Only `refill`, which runs inside `py.detach`, still takes locks plainly. Converted: `__next__`'s pop and its post-refill `is_empty`, `closed`, `rows`, and `close`/`__exit__` via `shut`. 2. Lock order is `inner` -> `prefixes` -> `pending`, with `inner` the only one ever held across another. `refill` now pulls a batch into a local queue and appends it to `pending` in one step, so another thread's `next()` waits only for the append rather than for a whole batch of Python `read()` calls. `inner` stays held across the append, which is what keeps two concurrent refills from interleaving their batches out of file order. `shut` takes the source out of the lock and drops it afterwards: the final decref of a file object can run a `__del__`, which must not happen while we hold `inner`. A source whose `read()` re-enters its own reader would still self-deadlock on the non-reentrant `inner`, so the pulling thread is recorded and `next()` and `close()` refuse re-entry with a `RuntimeError`; `closed` answers `False` without taking the lock at all. Regression tests: four threads draining one reader over a slow binary file object (1387 rows, 1387 distinct lines, daemon threads with a join timeout so a deadlock fails instead of hanging), `close()` racing an in-flight pull, and the re-entrancy cases in a subprocess under a timeout. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…release Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Claude-generated PR description
Python bindings Phase 2 — native API v2
Stacked on #20 (
python-phase0, Phases 0–1). This PR targetspython-phase0, notmain; review/merge #20 first. If #20 is squash-merged, rebase withgit rebase --onto main <python-phase0 tip at merge> python-phase2.Replaces the eager, positional, strings-only native API with a streaming reader, typed rows with column names, and a pure-Python eager
read()on top of it.Tickets (one commit each, plus two follow-ups)
5be857aFecError(ValueError)→FecParseError,MissingMappingError(row_type, version, line); missing path →FileNotFoundErrorwitherrno/filename77e0736Row: by name → typed, by position → raw; shared interned schema; slices, eq/hash, pickling,extra_fields,line; dropsItemization. AddsFilingRow.linetofec-parser7266a11open()→FilingReader: streaming, GIL released per 256-row batch,rows(*prefixes)filtered in Rust, context manager, frozenHeader/Cover, cover dates asdate89f0fd8test_gil_releasedmade duration-based so it holds on release builds601d2a4read()/Filingin pure Python overopen();rowscached,itemizationsdeprecated; pandas test + pandas in CI4d5684dTypeErrornaming'rb'for text-mode files592f2ceFilingReadermutex while holding the GIL (deadlock with threads sharing one reader over a file object). See# LockingonFilingReaderc9a3a0ccover_row= full cover line as a typedRow(tests + docs);fec_header()reads only theHDRrecord.FilingHeader::from_record→pubinfec-parser99ac25atest-python-freethreadedCI job (build againstcpython-3.14t, import under-W error::RuntimeWarning, pytest);test_shared_reader_across_threadsdf6892bbenchmarks/python/bench.py(subprocess per scenario),make bench,tests/test_perf.py(allslow)3d6a50etests/README.mdrewritten for the v2 APIfec-parserchanges are limited to the two noted above (FilingRow.line,from_recordvisibility).src/fecfile.rs(the compat layer) is untouched — Phase 3.Done-when numbers (ticket 19)
Apple M4 Pro, CPython 3.13.1, release build, 91 MB filing
1805248.fec(408,160 rows); reproduce withmake benchfromcrates/fec-py.open(), streamedFiling(path))open().rows("SB")open(bytes)read()pd.DataFrame(read(p).rows)fecfile.from_filefecfile.iter_filetest_open_streams_under_100mbasserts< 64 MBas a regression guard (100 MB remains the roadmap's done-when).test_background_thread_progresses_during_parseasserts ≥ 50 %, duration-based.pd.DataFrame(read(p).rows)→ shape(408160, 79);contribution_amountisfloat64;contribution_dateisobjectdtype holdingdatetime.date(pandas does not auto-convertdate;pd.to_datetime(df["contribution_date"])givesdatetime64)..rowsre-access is free (cached list): 20 accesses< 0.01 s.API breaks
Itemizationclass removed →Row.len(row)is now the mapped column count (len(row.fields())is the raw field count);iter(row)yields column names;row["name"]is typed (float/datetime.date/ rawstrif it doesn't parse /Noneif empty; text stays""),row[i]is the rawstr.Filingreplaced by the pure-PythonFiling/read();Filing.itemizationsis a deprecated alias of.rows(DeprecationWarning).Filing(id='1921705', form_type='F3N', filer_id='C00900860', 20 rows),Row(row_type='SA11AI', line=3, 45 fields),FilingReader(id=…, form_type=…, filer_id=…).Cover.coverage_from_date/coverage_through_date(andCover.fields()values) aredatetime.date | None; they were ISOstr.FecParseError/MissingMappingError(bothFecError(ValueError)) instead of bareValueError; missing path →FileNotFoundErrorwitherrno/filename(wasOSError(str)); eagerread()is strict on unmapped rows,open()raises per row and the reader stays usable (needs awhile/next()loop — aforloop can't catch-and-continue).fec_header(source, /): parameter renamed fromcontents, positional-only, accepts everyopen()input.stralways means path; text-mode files →TypeErrornaming'rb'.open,read,FilingReader,Row,cover_row,id,fec_version,rows(*prefixes), buffer/mmap/file-object inputs,Rowpickling/eq/hash, pandas viapd.DataFrame(filing.rows).Known issue found during final benchmarking — not fixed in this PR
Benchmarking against a 2.27 GB ActBlue filing (FEC-2009053, August Monthly 2026) exposed a pre-existing
fec-parserbug:Filing::from_reader(crates/fec-parser/src/lib.rs:265) builds itscsvreader without.quoting(false), so a field beginning with"opens a CSV quoted section that swallows every following line until the next". On that filing one record absorbed 337,348 lines (74.6 MB) into a single field:open()yielded 9,588,118 rows instead of 9,925,464 and peaked at 253 MB. With.quoting(false)applied experimentally (then reverted): 9,925,464 rows (=wc -lminus HDR and cover), 4.18 s, 35.5 MB peak — i.e. flat memory from 91 MB to 2.27 GB — vs. PyPIfecfile.iter_fileat 192.75 s / 37.7 MB. The bug also affects thefecCLI. It is outside Phase 2's allowed parser edits, so it is left for a decision: one line plus a regression test, either as a follow-up commit here (the same setting is needed infec_header()'s one-record reader) or as a separate PR againstmain. The 91 MB numbers above are unaffected (row count matchesfecfileto within its header/summary rows).Verification
Run on
python-phase2@3d6a50efromcrates/fec-py:make test→ 198 passed, 5 skipped (opt-innetwork/slow);make test-slow→ 4 passed;make stubs(stubtest + mypy) clean;make notebook-checkpasses (~1 s, no warnings, typed values in the native section);make benchas above;cargo clippy -p libfec_parserandcargo clippy -p fec-cliwarning-free;cargo test -p fec-parserok (the crate has no tests).crates/fec-py/README.mdwere run by hand againsttests/fixtures/1721696.fecand the commented values are real output.libfec_parser-0.0.32-cp314-cp314t-…whl; the module imports cleanly under-W error::RuntimeWarning(sogil_used = falsewas not needed); pandas has a 3.14t wheel; full suite 198 passed, 2 skipped. Thetest-python-freethreadedActions job iscontinue-on-error: trueand first runs on this PR.plans/andtodos/are local-only (untracked), so the Phase 2 benchmark table appended toplans/python/README.mdand its "Next" update don't appear in the diff.🤖 Generated with Claude Code
https://claude.ai/code/session_01V8jXKZ3DKfu41iLZhH71GY