Rename the opt-in S3FS CSV reader and define its protocol

[laughingman7743]

WHAT

Rename DefaultCSVReader to EmptyStringAsNullCSVReader for PyAthena 4.0 and remove the old name. AthenaCSVReader remains the default.
EmptyStringAsNullCSVReader now returns None for every empty field itself, so the result set no longer checks the reader's class. Cursor results for both readers are unchanged; direct iteration of the opt-in reader now yields None instead of ''.
Define the public CSVReader protocol (constructor, __iter__, __next__, close) and type csv_reader with it, so custom reader classes are typed without a closed union. Custom readers that worked before need no change.
Update all three S3FS cursor families, their tests, and the CSV/NULL handling and API documentation, including a 4.0 migration note.
Add tests/pyathena/s3fs/test_result_set.py for the result set's NULL handling with built-in and independent readers, CSV and TSV files, type hints, and managed results.

WHY

AthenaCSVReader is the default, so the old reader name misrepresents its role.
Keeping the '' -> None mapping in the reader makes the name match what the reader returns and leaves the result set with one contract: readers return None for NULL.

Closes #1097.

Release notes (4.0.0): DefaultCSVReader is renamed to EmptyStringAsNullCSVReader with no alias; iterating EmptyStringAsNullCSVReader directly returns None for empty fields.

TEST

Tested head: f41b8a07bd860ec94496f0c4c2eaafdf047c193e.

• just format and just lint: passed.
• uv run --env-file .env pytest -n 1 tests/pyathena/s3fs/test_reader.py tests/pyathena/s3fs/test_result_set.py: 48 passed (reader and result-set operations use in-memory streams and mocked AWS calls).
• uv run --env-file .env pytest -n 4 tests/pyathena/s3fs tests/pyathena/aio/s3fs: 122 passed, 1 skipped (the existing skip needs the insert_test table). Includes live Athena/S3 coverage of both readers for all three cursor families.
• A custom reader without extra attributes, which raised AttributeError on the previous head 010be3f7, returns rows on this head.
• just docs lint: passed.
• Self-review rounds one and two and an independent Codex review (gpt-6.1-sol, read-only, static only) are recorded inline as CLEAN.
• Ready-triggered Test workflow run 37956354739: Python 3.14, 2769 passed, 10 skipped, 13 warnings. Workflow path filters excluded the SQLAlchemy and Spark suites.

🤖 Generated with Claude Code

[laughingman7743]

Self-review round one — CLEAN (static implementation review).

Frozen range: 971dd12dd1a638429a5691d3be0e300555edb15e..146455fe6c59b4e0b87153b426b527d3afd10fd4. Initial pass covered all 13 changed files, including both reader implementations, the structural Protocol and alias, result-set conversion and closure, the sync/threaded/native-asyncio callers, regression tests, and changed docs/API references.

Traced CSV header skipping and tab-separated files, default NULL/empty-string preservation, opt-in normalization before hinted and unhinted conversion, inherited and independently implemented readers, batches/EOF/rownumber, stream closure, managed-results fallback, and pandas' use of the unchanged raw Athena reader. No actionable defects found.

Author validation: required lint passed; reader/result-set offline tests 57 passed; three cursor-family forwarding tests 6 passed. Reinstating the original class-identity _fetch() in an isolated process (renamed import adapted) produced 9 failing regression cases. AWS coverage is pending and is not established by this static review.

[laughingman7743]

Round-one repair follow-up — CLEAN for the corrected scope.

The additional fixture-path audit found the async/aio test-option defect in the original head; the independent reviewer separately confirmed it. This supersedes the original CLEAN conclusion for those test parametrizations. Repair: 774219e113d55e6bc4e54c290e7351e7dcfac433; complete frozen range: 971dd12dd1a638429a5691d3be0e300555edb15e..774219e113d55e6bc4e54c290e7351e7dcfac433. Both old objects were verified and the bounded range-diff 971dd12dd1a638429a5691d3be0e300555edb15e..146455fe6c59b4e0b87153b426b527d3afd10fd4 versus 971dd12dd1a638429a5691d3be0e300555edb15e..774219e113d55e6bc4e54c290e7351e7dcfac433 contains only the two fixture corrections and the custom-reader migration clarification.

Traced both indirect fixtures through connect/aio_connect, Connection.cursor_kwargs, cursor creation, and the shared result set: default cases retain AthenaCSVReader; explicit cases now deliver the selected reader. A mocked-session check proved that the former top-level option was ignored and the repaired cursor_kwargs option is applied, with no AWS calls. Existing fixture teardown and expectations are retained. Protocol/reader/result-set code is unchanged. Required format/lint and documentation lint/render checks passed. Live AWS validation remains pending.

[laughingman7743]

Final author runtime validation for 774219e113d55e6bc4e54c290e7351e7dcfac433:

• Local required format/lint and documentation checks passed. Targeted live AWS S3FS suites across all three cursor families: 133 passed, 1 pre-existing skip requiring insert_test.
• Ready-triggered Test workflow 37554213131, Python 3.14: 2767 passed, 10 skipped; all applicable PR checks completed successfully. The job log confirms skip-spark=true and skip-sqla=true, so those packages and compliance suites are outside this workflow selection.
• The published head still matches the self-reviewed and independently reviewed repaired revision. PR state is Ready, MERGEABLE, and CLEAN. The dedicated worktree and main checkout are clean.

These are author/CI execution results; the independent Claude reviews remain source-only. No merge was performed.

[laughingman7743]

Round-one test-organization follow-up — CLEAN.

Frozen full range: 971dd12..010be3f; previously reviewed head: 774219e. Both old objects were verified; the bounded patch-series comparison adds only the test-class organization.

Covered all three S3FS result-set tests, their seven/six/two reader parametrizations, CSV/TSV cross-product, type-hinted conversion, managed/API fallback, fetch/EOF state, and stream cleanup. Each test body and decorator is AST-identical to the prior version after removing the added self argument. The helpers remain module-scoped; the test class has no constructor, state, or setup hooks. No production code changed. just format and just lint passed; the existing reader/result-set suites passed all 57 cases. No actionable findings in this follow-up scope.

[laughingman7743]

Current-head completion evidence for the test-class follow-up.

Reviewed and validated head: 010be3f. Both author review perspectives and the requested independent claude-opus-5-5 / first-party Max follow-up are CLEAN for the added test-organization scope. Local reader/result-set validation: 57 passed. Format and lint passed. Production code remains identical to the previously reviewed head 774219e.

All applicable current PR checks passed. Ready-triggered Test workflow run 37949079000 completed successfully on this head; its Python 3.14 job 113882910252 reported 2780 passed, 10 skipped, 13 warnings. The workflow logs confirm skip-spark=true and skip-sqla=true for this S3FS-only change. Documentation lint/build and the other offline checks also passed. The PR is Ready and mergeable; no merge was performed.

[laughingman7743]

Self-review round one for the reader-side conversion repair: CLEAN (implementation behavior).

Range: 971dd12dd1a638429a5691d3be0e300555edb15e..f41b8a07bd860ec94496f0c4c2eaafdf047c193e; repair commit f41b8a07bd860ec94496f0c4c2eaafdf047c193e on top of the previously reviewed head 010be3f740c587c9f86f5f8743d0388fa52f19ae. The repair narrows the contract rather than expanding it, but the protocol changed, so this pass covered the full range.

Design change, agreed with the maintainer after a consultation with Claude Code claude-fable-5-1 (Max profile, effort high, read-only plan mode): keep the names AthenaCSVReader / EmptyStringAsNullCSVReader, and move the '' -> None mapping into EmptyStringAsNullCSVReader.__next__. This removes empty_strings_as_null from both readers and from the CSVReader protocol, and removes the result-set branch. The class name now describes what direct iteration returns, and the result set has a single contract: readers return None for NULL.

Traced: header skipping (next() in _init_csv_reader), CSV and TSV paths, the empty-line normalization ([None], previously [''] then mapped to None), hinted and unhinted conversion, managed/API fallback (reader unused), and all three cursor families, which pass the class through unchanged. Cursor results for both built-in readers equal the previous head's. A probe custom reader without the removed attribute raised AttributeError on 010be3f7 and returns rows on f41b8a07.

Tests: the reader tests now expect None for empty fields; the result-set tests drop the flag-only reader variants and keep the independent tuple reader, which also covers a custom reader with no extra attribute. The explicit AthenaCSVReader case in the async and aio integration tests duplicated the default case and was removed, saving two Athena queries per run.

Validation on f41b8a07: just format, just lint passed; uv run --env-file .env pytest -n 1 tests/pyathena/s3fs/test_reader.py tests/pyathena/s3fs/test_result_set.py: 48 passed; uv run --env-file .env pytest -n 4 tests/pyathena/s3fs tests/pyathena/aio/s3fs: 122 passed, 1 skipped (the existing insert_test skip); just docs lint passed.

[laughingman7743]

Self-review round two — CLEAN (compatibility, claims, and operational review).

Frozen range: 971dd12dd1a638429a5691d3be0e300555edb15e..146455fe6c59b4e0b87153b426b527d3afd10fd4. Initial pass covered all 13 changed files and the PR description, with a separate audit of defaults, caller compatibility, public typing, migration examples, NULL handling guarantees, and validation claims.

Confirmed that the old public name is removed for 4.0 and appears only in migration prose; AthenaCSVReader remains the default; direct reader iteration retains its existing values; the instance flag governs CSV/TSV results across all three cursor families, including the type-hint branch. Managed/API fallback retains its existing values and is explicitly excluded from the flag's documented scope. Positive and expected-negative mypy probes support the structural typing claim; documentation lint, full multiversion build, direct current-tree build, and rendered migration/API checks passed. Source inspection does not establish AWS execution.

No additional Athena/S3 requests, retry changes, dependency changes, or unrelated dialect changes are introduced. Existing Sphinx warnings are outside this patch. Shared AWS validation is serialized with other runs; live coverage remains pending. No actionable defects found.

[laughingman7743]

Round-two repair follow-up — CLEAN for the corrected scope.

Frozen range: 971dd12dd1a638429a5691d3be0e300555edb15e..774219e113d55e6bc4e54c290e7351e7dcfac433; reviewed the bounded patch-series comparison from 146455fe6c59b4e0b87153b426b527d3afd10fd4 and traced both corrected fixture mappings to their existing connection/cursor contracts. The updated migration note explicitly states that custom readers need empty_strings_as_null, with False preserving empty strings and True normalizing them before result-file type conversion. The new wording rendered successfully and matches the Protocol and actual converter path. Direct iteration and API fallback remain distinct in the docs.

Clarification to the initial operational record: library execution adds no Athena/S3 requests; the integration coverage adds six query cases across the two async cursor families. No retry, dependency, or dialect change is introduced. Repaired test expectations remain based on literal observable values. Static/source checks and mocked forwarding do not establish live AWS results. No further actionable defects found within the repaired scope.

[laughingman7743]

Round-two test-organization follow-up — CLEAN.

Frozen full range: 971dd12..010be3f; previously reviewed head: 774219e. Audited the claim that grouping tests preserves coverage and runtime semantics separately from the implementation pass.

The 22 result-set parameter cases, explicit expected values, context-manager cleanup and reader selection are unchanged; the combined reader/result-set run collected and passed 57 cases. The new class matches AGENTS.md's rule permitting unit tests to group one object's behavior. Pytest creates separate test instances; no class-scoped fixtures or mutable self state were added. Searching the repository found no references to the former standalone test node names; only the class prefix changes. Live session setup/teardown from tests/pyathena/conftest.py ran under the standard pytest command; the tested reader/result-set operations themselves use in-memory streams and mocked AWS calls. The prior broad AWS CI result remains evidence for 774219e; current-head CI will be required before Ready. No new API, naming, documentation or performance claims were introduced.

[laughingman7743]

Self-review round two for the reader-side conversion repair: CLEAN (compatibility, claims, operations).

Range: 971dd12dd1a638429a5691d3be0e300555edb15e..f41b8a07bd860ec94496f0c4c2eaafdf047c193e.

Compatibility: cursor results are unchanged from 3.x for both readers. The only behavior change beyond the rename is direct iteration of EmptyStringAsNullCSVReader, which now yields None instead of ''; the 4.0 note in docs/s3fs.md states it. Custom readers no longer need a new attribute, so the earlier migration requirement is removed from the docs. Release notes for 4.0.0 should list the rename and the direct-iteration change.

Claims: removed the doc sentences that described the flag and the result-set conversion, and the claim that direct iteration returns empty strings. The custom-reader section now states that the cursor treats None as NULL and that managed query results are read through the Athena API, which matches AthenaS3FSResultSet.__init__. docs/api/s3fs.rst still documents the protocol and both readers.

Operational: the change removes one list comprehension per row from the default reader path and moves the same comprehension into the opt-in reader; the integration tests issue two fewer Athena queries. Upstream master advanced to f3532e9fb57fd769441e533f0b8375fd854ece69 (#1110); it touches no S3FS, reader, or docs surface in this PR, and GitHub reports the PR mergeable.

[laughingman7743]

Relayed independent review — FINDINGS (source inspection only).

Reviewer: Claude Code claude-opus-5-5, first-party Max subscription/profile, effort high; session ee9a320b-7504-4422-be17-6da5f2619913. Frozen range: 971dd12dd1a638429a5691d3be0e300555edb15e..146455fe6c59b4e0b87153b426b527d3afd10fd4.

The complete diff and all 13 changed files were reviewed with directly related public source/tests and repository conventions. Read/Glob/Grep only; no edits, commands, builds, tests, PR discussion, credentials, personal memory, or GitHub writes. The frozen snapshot was verified unchanged. Actual model usage identified claude-opus-5-5.

[Medium] tests/pyathena/s3fs/test_async_cursor.py:32-33 and tests/pyathena/aio/s3fs/test_cursor.py:27-28 pass csv_reader as a connection setting through their fixtures. Connection.cursor() uses cursor_kwargs, so these settings are dropped and the opt-in cases use AthenaCSVReader: the query returns (None, '', 'text') instead of the expected (None, None, 'text'). Wrap reader options inside cursor_kwargs. Verified by tracing both fixtures and Connection and by a mocked-session forwarding check with no AWS calls.

The reviewer found no defect in the library change, raw reader behavior, hinted/unhinted conversion, managed/API fallback, closure, three cursor callers, the offline result-set tests, or the API documentation. A non-blocking observation recommends mentioning the newly required custom-reader flag in the 4.0 migration note. We retain the explicit Protocol requirement and will document it; no fallback behavior is added.

The author also found the fixture issue during additional source checks while this frozen review was running. The author's worktree has a pending correction, so this review applies to the original published head only; the corrected head requires both self-review follow-ups and an independent follow-up before Ready. Runtime AWS coverage is not established by this review.

[laughingman7743]

Fixed in 774219e113d55e6bc4e54c290e7351e7dcfac433: both indirect fixture parameters now nest csv_reader under cursor_kwargs. Verified the connection/cursor forwarding with a mocked session and no AWS calls. Added the custom-reader flag requirement to the 4.0 migration note rather than introducing an implicit fallback.

Both self-review perspectives rechecked the narrow repair; format/lint and documentation lint/render checks passed. A fresh bounded Claude Max follow-up on this published head is still required before Ready. Live AWS validation remains pending.

[laughingman7743]

Relayed independent repair follow-up — CLEAN (static review only).

Reviewer: Claude Code claude-opus-5-5, verified first-party Max subscription/profile, effort high; session e94b8e0d-4197-4098-b30a-926f44509588. New frozen range: 971dd12dd1a638429a5691d3be0e300555edb15e..774219e113d55e6bc4e54c290e7351e7dcfac433; old range ended at 146455fe6c59b4e0b87153b426b527d3afd10fd4.

The reviewer inspected the verified patch-series comparison and all three repaired files, tracing both indirect fixtures through connect/aio_connect, Connection's cursor_kwargs merge, both async cursor constructors, and the shared result set. The corrected reader selections and literal NULL/empty-string expectations are consistent. The new migration sentence accurately requires the custom-reader flag and scopes it to result files rather than managed/API fallback. No further actionable defects found within the repaired scope.

Read/Glob/Grep only, no edits, builds, tests, commands, GitHub writes, credentials, PR discussion, personal memory, or delegation. Model usage confirmed claude-opus-5-5; permission denials were empty. The final review snapshot and author worktree were verified unchanged. Together with the completed initial full review, this resolves the fixture finding. Runtime Athena/S3 validation is still separate and running; it was not performed by the independent reviewer.

[laughingman7743]

Relayed independent test-organization follow-up — CLEAN (static inspection only).

Reviewer: Claude Code, actual model usage claude-opus-5-5, first-party Max profile, effort high. Session: 6f4e8028-44e2-423f-8d27-7f97be624441. Max authentication was reverified at launch; no Enterprise/API-key/provider override was used. The first attempt was quota-limited; the completed follow-up ran after the reset.

Frozen full range: 971dd12..010be3f. Previously reviewed head: 774219e. Reviewed the literal one-file revision and bounded patch-series comparison with eight public tracked source/convention files. Snapshot hashes and the PR worktree were unchanged.

The reviewer found all 14 CSV/TSV, 6 type-hinted, and 2 managed/API parameter cases preserved, with the same inputs, explicit expected values, marks, mock/stream construction, and cleanup. TestAthenaS3FSResultSet has no constructor or shared self state; its methods add only self. Grouping follows AGENTS.md and the sibling reader tests. No actionable findings were reported.

Limits: no tests, builds, commands, or GitHub writes were executed by the reviewer; unchanged production behavior, base-commit contents and pytest configuration were not re-reviewed. The reviewer noted the new class segment in full pytest node IDs. Author verification confirms this test file is newly added in this PR (status A against the merge-base), its parameter IDs are unchanged, repository pytest discovery permits the class, and no references to the former standalone node names exist. The current author-run reader/result-set suite collected and passed all 57 cases.

[laughingman7743]

Relayed independent review of the reader-side conversion repair: CLEAN (static review only).

Reviewer: Codex CLI 0.161.0, model gpt-6.1-sol, codex exec -s read-only; session 01a1215f-931e-7ba2-b72e-0369fe50dd73. The repair was authored by Claude Code claude-opus-5-5, so a different model reviewed it. Frozen range: 971dd12dd1a638429a5691d3be0e300555edb15e..f41b8a07bd860ec94496f0c4c2eaafdf047c193e, full range rather than a range-diff. The review ran in a clean detached snapshot of f41b8a07, which was verified unchanged afterwards. The prompt omitted the PR number, description, prior findings, and author conclusions. The reviewer did not run builds, tests, the type checker, GitHub, or network access.

Covered: all 13 changed files; both readers and the result-set conversion (CSV header, headerless TSV, empty lines, single-column NULL rows, type-hint branches); managed-result API fallback and resource closure; CSVReader, CSVReaderType, custom-reader compatibility, and forwarding in all three cursor families; test placement, explicit expectations, fixtures, cleanup, AWS cost; changed docs, API references, docstrings, and old-name removal.

Result: no actionable defects. Cursor results for both built-in readers remain equivalent to the base, including type-hinted conversion.

Non-actionable observations and disposition:

• The added tests do not combine TSV with type hints. Not added: TSV and CSV differ only in delimiter and header skipping, both covered by test_reader_null_conversion, and the type-hint branch does not depend on either.
• The four new constant-query integration cases (two per async/aio family) add Athena executions. Kept: they are the only tests that pass csv_reader end to end through the async and aio cursor fixtures; the duplicate explicit AthenaCSVReader case was already removed in f41b8a07.