Apply dict_type only to the dict cursor it was given to

[laughingman7743]

WHAT

DictCursor, AsyncDictCursor and AioDictCursor now apply dict_type only to the result sets of the cursor it was given to, by passing it to each result set as a constructor argument.

• AthenaDictResultSet.__init__ takes an optional keyword-only dict_type; when it is not None, it sets dict_type on that instance before the first page is read. The class attribute stays the default.
• AthenaAioResultSet.create() forwards additional keyword arguments to the constructor.
• Cursor, AsyncCursor and AioCursor keep _result_set_kwargs (empty by default) and pass it to each result set they create.
• The dict cursors take dict_type as an explicit keyword argument and, when it is not None, put it in _result_set_kwargs. A cursor subclass that assigns its own AthenaDictResultSet subclass to _result_set_class gets dict_type applied as well, as long as that class's __init__ accepts dict_type (it does unless the subclass overrides __init__ without forwarding keyword arguments).
• The __init__ docstrings added in Document every public API in pyathena/ and check docstrings with ruff #919 are updated.

Behavior changes for the 4.0.0 release notes:

• dict_type passed to one dict cursor no longer changes the row type of other dict cursors. Code that relied on this must pass dict_type (or cursor_kwargs={"dict_type": ...}) to each cursor.
• An explicit dict_type=None now uses the default (dict, or a value assigned to AthenaDictResultSet.dict_type) instead of assigning None to the class attribute, which made rows fail to build in every dict cursor.
• A result set subclass whose __init__ does not accept dict_type raises TypeError when its cursor is given dict_type. Before, dict_type reached such a class through the shared class attribute. Cursors without dict_type pass no extra argument.

Setting AthenaDictResultSet.dict_type directly still applies to dict cursors created without dict_type.

WHY

Closes #923.
The constructors assigned dict_type to the class attribute of the shared result set class, and rows are built from that attribute on each fetch.
One cursor's dict_type therefore changed the row type of every dict cursor in the process, including cursors created before it.
The result set reads the first page while it is created (in its constructor, or in create() for the aio result set), so the cursor passes the type when it creates the result set.

TEST

Tested commit: ee872f7.

• just lint: passed.
• New test_dict_type in TestDictCursor, TestAsyncDictCursor and TestAioDictCursor: opens a second cursor with dict_type=OrderedDict on the fixture's connection and checks that it returns OrderedDict while the fixture cursor still returns dict. All three failed on the original code (assert <class 'collections.OrderedDict'> is dict).
• New test_dict_type_custom_result_set in the same classes: a cursor subclass that assigns its own result set subclass after __init__ gets OrderedDict rows from that class.
• uv run --env-file .env pytest -n 1 tests/pyathena/test_cursor.py::TestDictCursor tests/pyathena/test_async_cursor.py::TestAsyncDictCursor tests/pyathena/aio/test_cursor.py::TestAioDictCursor: 16 passed (also on e27e6e2, with tests/pyathena/aio/test_result_set.py: 4 passed).
• The issue's reproduction without AWS, extended to the three cursor classes: AthenaDictResultSet.dict_type and AthenaAioDictResultSet.dict_type stay dict, and a dict cursor without dict_type passes no extra argument to its result set.
• Not run locally: the full just test pyathena suite; it runs in CI once the PR is Ready.

🤖 Generated with Claude Code

[laughingman7743]

Self-review round one (implementation behavior) — FINDINGS, repaired

Scope: base 6be315ec4c87758f993778b0b2167696e848390a (merge-base with master) .. head 0b236dd9ace2834ab680e3726b4cfd2d4b8a34b1, full diff (3 cursor modules, 3 test modules).

Covered:

• Behavior: the only constructions of _result_set_class are Cursor.execute (pyathena/cursor.py:188), AsyncCursor._collect_result_set (pyathena/async_cursor.py:194) and AioCursor.execute via create() (pyathena/aio/cursor.py:170); all instantiate through the class, so the subclass inherits construction, pre-fetch and create() unchanged. No code compares result set types by identity; isinstance checks still hold.
• Shared state: cursors without dict_type keep the base class, which is no longer modified (checked with the issue's no-AWS reproduction for all three cursor classes; AthenaAioDictResultSet no longer inherits a value written to AthenaDictResultSet by a sync cursor).
• Resources: the generated classes are collected once their cursor is gone (checked with weakref + gc.collect()); ABCMeta is applied via the base's metaclass, __abstractmethods__ is empty.
• Tests: each new test_dict_type failed on the unfixed code (assert <class 'collections.OrderedDict'> is dict) and asserts the observable row type of a cursor created before the dict_type cursor.

Finding (introduced, repaired in 27626643): classes created with type() under ABCMeta took __module__ from the abc module, so repr() showed <class 'abc.AthenaDictResultSet'>. The namespace now passes the base class's __module__; repr is pyathena.result_set.AthenaDictResultSet / pyathena.aio.result_set.AthenaAioDictResultSet. Lint and the three dict cursor test classes (13 passed) re-run on the repair.

Not changed: the 10-line block is repeated in the three cursors; a shared helper on the result set class would be a new method, outside the approved shape of this fix.

[laughingman7743]

Self-review round two (claims, callers, operations) — FINDINGS, PR description corrected

Scope: base 6be315ec4c87758f993778b0b2167696e848390a .. head 27626643b1770bd9cc143c3a5f1486c018475f3e, full pass over the PR body, the three changed docstrings, both commit messages, docs and issue premises.

Claims checked:

• "rows are built from that attribute on each fetch" / "including cursors created before it": AthenaDictResultSet._get_rows reads self.dict_type per page (pyathena/result_set.py:815); each new test's fixture cursor is created before the dict_type cursor and failed on the unfixed code.
• cursor_kwargs={"dict_type": ...} path: Connection.cursor merges {**self.cursor_kwargs, **kwargs} and forwards the remainder as **kwargs, so the dict cursors see dict_type exactly as for a direct argument; unrecognized keys still end in BaseCursor.__init__(**kwargs) as before.
• Docstrings ("other cursors are not affected") and docs: docs/cursor.md:76,85,284,293 and docs/aio.md:192 pass dict_type per cursor and stay correct; no prose describes the old global effect.
• Existing callers: same trigger ("dict_type" in kwargs, including an explicit None, which now breaks only that cursor instead of all of them); a global AthenaDictResultSet.dict_type = ... still applies to cursors without dict_type, now also to AthenaAioDictResultSet, which is no longer shadowed by an aio cursor's assignment.
• Concurrency: no shared mutable state remains, so threads creating dict cursors with different dict_type no longer race.
• AWS: no runtime API change; the tests add 2 one_row queries per dict cursor type.

Findings (PR description, corrected):

1. "subclassing the result set class still works as before" was too broad. A DictCursor subclass that replaces _result_set_class after DictCursor.__init__ previously received dict_type through the shared class attribute and no longer does. The body now states this in the behavior-change note.
2. "The result set reads the first page in its constructor" did not hold for aio (AthenaAioResultSet.create() pre-fetches); reworded.

Evidence limits: local runs cover the three dict cursor test classes (13 passed on 27626643) and lint; the full just test pyathena matrix runs in CI after Ready.

[laughingman7743]

Independent review (relayed) — FINDINGS

Reviewer: Codex CLI 0.160.0, model gpt-6-astra, reasoning effort max, sandbox read-only, --ephemeral, session 01a0ffc3-91e5-7322-8076-eb77dba8a8a7. Static review only (no builds, tests or network). Range: merge-base 6be315ec4c87758f993778b0b2167696e848390a .. head 27626643b1770bd9cc143c3a5f1486c018475f3e, detached snapshot without .env; the prompt carried the diff and intended behavior, not the PR number, description, commits or earlier findings. Snapshot and PR worktree verified unchanged after the run.

Reviewer output (verbatim)

Surfaces covered:

• Direct keywords and connection cursor_kwargs, including override precedence.
• Sync pre-fetch, thread-pool execution/description, asyncio create(cls), MRO, and pagination.
• Cursor/result-set subclasses, global defaults, and explicit dict_type=None.
• Dynamic-class ABC/metaclass behavior, repr, garbage collection, and typing.
• Changed docstrings and tests. All three tests assert observable row types and would fail on the old code’s final is dict assertion.

FINDINGS

1. P2 — Regression: custom result-set selection loses dict_type.
   Locations: pyathena/cursor.py:230, pyathena/async_cursor.py:349, pyathena/aio/cursor.py:260.

   An existing subclass can select its result-set implementation after initialization:

   class CustomResultSet(AthenaDictResultSet):
       pass
   
   class CustomCursor(DictCursor):
       def __init__(self, **kwargs):
           super().__init__(**kwargs)
           self._result_set_class = CustomResultSet

   With the global default still dict, creating this cursor with dict_type=OrderedDict and executing SELECT 1 AS a now returns a plain dict. Previously, CustomResultSet inherited the configured OrderedDict from the base class.

   The option now exists only on the generated class, which the subclass replaces. Equivalent thread-pool and asyncio subclasses have the same regression. Preserve the cursor’s option and apply it to the selected result-set implementation before pre-fetch. The added tests do not cover this case.

Pre-existing behavior: explicit dict_type=None still raises TypeError when constructing a row; it did not previously mean “use the default.”

Source inspection only; no builds, tests, mypy, modifications, or network access.

Author verification: Finding 1 is reproducible by reading the code: a DictCursor subclass that assigns _result_set_class after super().__init__() replaces the generated class, so dict_type is lost, in all three cursors. It is the same case recorded in round two and in the PR body's behavior-change note. Status: awaiting the maintainer's decision between accepting it as a documented behavior change (it depends on replacing the private _result_set_class) and extending the fix to apply the cursor's dict_type to whichever class is selected, which would touch the result set construction paths. The dict_type=None note is pre-existing behavior, not a regression.

[laughingman7743]

Repair (maintainer chose to apply dict_type to a replaced class) — b2ece224, 2ee5fbc0 on top of 27626643; range-diff over merge-base 6be315ec4c87758f993778b0b2167696e848390a shows the first two commits unchanged.

• pyathena/cursor.py, pyathena/async_cursor.py, pyathena/aio/cursor.py: each dict cursor stores self._dict_type before super().__init__() and overrides _result_set_class with a property (@override, explicit-override false positive when overriding property with setter python/mypy#15900 ignore as for arraysize). The setter wraps any assigned AthenaDictResultSet subclass in a subclass carrying dict_type (and __module__); other classes are stored as is. The base cursors' own assignments (AthenaResultSet / AthenaAioResultSet) pass through unwrapped. No change to result sets, execute() or base cursors.
• Explicit dict_type=None now means the default type (previously None was assigned to the shared class attribute). Docstrings updated in 2ee5fbc0; PR body lists both behavior changes.
• Tests: test_dict_type_custom_result_set per cursor type (subclass assigns its own result set subclass after __init__, asserts isinstance and OrderedDict rows); all three failed on 27626643 with assert <class 'dict'> is OrderedDict. On b2ece224: just lint passed, the three dict cursor test classes 16 passed, no-AWS reproduction keeps the shared classes at dict.

Self-review of the repair, round one (behavior): callers of _result_set_class are unchanged (pyathena/cursor.py:188, pyathena/async_cursor.py:194 from the executor thread, pyathena/aio/cursor.py:170 via create()), and all now read the property. Re-assignment wraps again (a subclass of a subclass), which is harmless. Noted, not changed: a subclass that assigns _result_set_class before super().__init__() now gets AttributeError because _dict_type is not set yet; that assignment was always overwritten by DictCursor.__init__, so it had no effect before either.
Round two (claims): the __init__ docstrings said "If they include dict_type", which no longer held for an explicit None → corrected in 2ee5fbc0 (lint only). PR body claims ("without changing the result set, execute() or base cursor code", the None behavior change, the tested commit) re-checked against the diff.

An independent follow-up review of the repair is next.

[laughingman7743]

Independent follow-up review (relayed) — CLEAN

Reviewer: Codex CLI 0.160.0, model gpt-6-astra, reasoning effort max, sandbox read-only, --ephemeral, session 01a0ffd2-b6a5-7a61-97d4-058d374fbcc2. Static review only. Scope: follow-up diff 27626643b1770bd9cc143c3a5f1486c018475f3e..2ee5fbc006b51c41a29d4ef9635228bc72bb9c85 (range-diff over merge-base 6be315ec4c87758f993778b0b2167696e848390a shows the earlier commits unchanged) plus the affected contracts, in a detached snapshot of 2ee5fbc0 without .env. Snapshot and PR worktree verified unchanged after the run.

Reviewer output (verbatim)

Surfaces covered:

• Every _result_set_class reader and writer, including base initialization order, synchronous execution, thread-pool collection, and asyncio creation.
• Property/MRO behavior on all three dict cursors, subclass reassignment, shared-class isolation, cursor_kwargs precedence, and explicit dict_type=None.
• Mypy annotations, override markers, changed docstrings, and existing callers.
• All three new tests: each asserts the custom result-set subclass and observable fetched-row type. Without the follow-up, its OrderedDict assertion would fail.

CLEAN

The earlier subclass-reassignment finding is resolved. No actionable regressions or additional in-scope pre-existing findings identified.

Static review only; builds, tests, and mypy were not run.

[laughingman7743]

Self-review round one (implementation behavior), rewrite — FINDINGS, repaired

The maintainer replaced the per-cursor subclass / _result_set_class property design with plain argument passing (e27e6e20), so this round is a full pass, not a narrow follow-up.
Scope: base 6be315ec4c87758f993778b0b2167696e848390a .. head e27e6e206520264a6e5ada11cfed553c50e37134, full diff (5 source, 3 test modules).

Covered:

• Construction paths: Cursor.execute passes 5 positional arguments plus keywords, AsyncCursor._collect_result_set keywords only, AioCursor.execute via create(), which now forwards **kwargs. AthenaDictResultSet.__init__ sets the instance dict_type before super().__init__(), so the sync pre-fetch in AthenaResultSet.__init__ already uses it; aio pre-fetches in create() after construction.
• MRO: for AthenaAioDictResultSet the next __init__ is AthenaAioResultSet.__init__ (5 positional + result_set_type_hints), which receives no dict_type because it is consumed first.
• Base cursors: _result_set_kwargs defaults to {}; non-dict cursors construct result sets exactly as before.
• Shared state: no class attribute is written; the no-AWS reproduction keeps AthenaDictResultSet.dict_type / AthenaAioDictResultSet.dict_type at dict.
• Tests: test_dict_type (failed on the original code) and test_dict_type_custom_result_set; 16 passed in the three dict cursor classes, 4 in tests/pyathena/aio/test_result_set.py.

Finding (introduced, repaired in ee872f7a): the dict cursors always passed dict_type (also None), so a user AthenaDictResultSet subclass with a fixed __init__ signature would raise TypeError even for cursors created without dict_type. They now add it to _result_set_kwargs only when it is not None; lint and the 16 tests re-run on ee872f7a.

[laughingman7743]

Self-review round two (claims, callers, operations), rewrite — FINDINGS, PR description corrected

Scope: base 6be315ec4c87758f993778b0b2167696e848390a .. head ee872f7a91da39ff2e60969718c57366a723ee6e, full pass over the rewritten PR body, the changed docstrings (AthenaDictResultSet.__init__, AthenaAioResultSet.create, three dict cursor __init__s), commit messages and docs.

Claims checked:

• "sets dict_type on that instance before the first page is read": true for sync (set before super().__init__(), which pre-fetches) and aio (create() pre-fetches after the constructor). The AthenaDictResultSet.__init__ docstring originally said it fetches the first page, which is false for aio; reworded in e27e6e20 before commit.
• cursor_kwargs and direct arguments: Connection.cursor passes everything as keywords, so the new explicit dict_type parameter receives both; nothing passes cursor arguments positionally.
• Global default: a value assigned to AthenaDictResultSet.dict_type still applies to cursors without dict_type, including aio through inheritance.
• Docs: docs/cursor.md and docs/aio.md examples pass dict_type per cursor and stay correct.
• AWS: no API change; the tests add one or two one_row queries per test.

Findings (PR description, corrected):

1. "A cursor subclass that assigns its own AthenaDictResultSet subclass ... gets dict_type applied" holds only if that class's __init__ accepts dict_type; now qualified.
2. Missing compatibility note: a result set subclass whose __init__ does not accept dict_type now raises TypeError when its cursor is given dict_type (before, the class attribute reached it). Added to the release-note list; cursors without dict_type are unaffected after ee872f7a.

Independent review of this revision is next.

[laughingman7743]

Independent review (relayed), rewrite — CLEAN

Reviewer: Codex CLI 0.160.0, model gpt-6-astra, reasoning effort max, sandbox read-only, --ephemeral, session 01a0fff0-6833-72c2-a3c5-6e9e897e0f6e. Static review only (no builds, tests, mypy or network). Scope: full range merge-base 6be315ec4c87758f993778b0b2167696e848390a .. head ee872f7a91da39ff2e60969718c57366a723ee6e in a detached snapshot without .env; the prompt carried the diff and intended behavior, not the PR number, description, commits or earlier findings. Snapshot and PR worktree verified unchanged after the run.

Reviewer output (verbatim)

Surfaces covered:

• Sync execute, thread-pool _collect_result_set (including description), asyncio create, repeated execution, and pagination.
• _result_set_class and _result_set_kwargs, direct constructor arguments, connection cursor_kwargs precedence, and dict_type=None.
• Cooperative initialization through AthenaAioDictResultSet’s MRO; row type assignment occurs before synchronous or asynchronous pre-fetch.
• Cursor isolation, preservation of shared classes, and custom result-set subclasses.
• Arrow, Pandas, Polars, S3FS, and Spark inheritance/construction paths; existing result-set callers.
• Constructor compatibility, mypy annotations/configuration, and changed docstrings, inspected statically.
• All six new tests and their fixtures. The three isolation tests would fail on the old code. The three custom-result-set tests would also pass on the old code, but assert observable subclass selection and fetched row types.

CLEAN

No actionable regression identified in the supplied range. Builds, tests, and mypy were not run; no network access was used. The checkout remains clean at the requested head.

Author note: the reviewer's observation that test_dict_type_custom_result_set would also pass on the original code is correct (the shared class attribute reached the subclass there); it guards the subclass path of the new argument passing, while test_dict_type is the regression test for #923.