Skip to content

Upgrade the locked data libraries and type-check per interpreter #822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
laughingman7743 merged 3 commits into from
Sep 25, 2026
+513 −305
Merged

Upgrade the locked data libraries and type-check per interpreter #822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b2f0a8c
Upgrade the locked data libraries and type-check per interpreter
laughingman7743 Sep 25, 2026
9af4a07
Expect the installed pandas string semantics in PandasCursor tests
laughingman7743 Sep 25, 2026
e3aa3df
Document that PandasCursor string NULLs follow the installed pandas
laughingman7743 Sep 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions docs/null_handling.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ based on actual testing with Athena:
| `Cursor` (default) | Athena API | `''` | `None` | ✅ Yes |
| `DictCursor` | Athena API | `''` | `None` | ✅ Yes |
| `PandasCursor` | CSV file | `NaN` | `NaN` | ❌ No |
| `PandasCursor` + unload | Parquet file | `''` | `None` | ✅ Yes |
| `PandasCursor` + unload | Parquet file | `''` | `NaN` (pandas 3) or `None` (pandas 2) | ✅ Yes |

@laughingman7743 laughingman7743 Sep 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review (relayed) — reviewer: Codex CLI 0.156.0, model gpt-6-sol, codex exec --sandbox read-only, session 01a0d905-5e80-77b2-9867-1f8077490dbb; static source review (no tests run); base 0c26c7ece28b67a06d538e69f9732df3ea1eba42, head 9af4a07642e4bc2181fea375fcdc642fab4fe19d; verdict FINDINGS (1). The review snapshot was unchanged afterward.

P2 (uv.lock:1256, tests/pyathena/pandas/test_cursor.py:1204 at 9af4a07): "The new lock selects pandas 3 on Python 3.11+, and the revised test accepts NaN for a SQL NULL returned by PandasCursor with unload=True. [...] An existing caller that checks cursor.fetchone()[0] is None for a nullable string will get a false result in the newly locked environment. The documentation still shows that result as None in docs/null_handling.md:180. Introduced by this diff for locked development and benchmark environments; the pre-existing dependency range already allowed users to install pandas 3 independently. Preserve the tuple API's NULL value or document and test the changed contract."

Verified; resolved by documenting in e3aa3df (maintainer decision: follow pandas). Keeping None in PyAthena would need either object conversion of string columns (a copy and a memory regression for large results), a process-wide pandas option inside a thread-pool path, or NaN-to-None mapping that cannot tell NULL from a real double NaN. This table row, the UNLOAD example, and the following text now state the pandas 2 and pandas 3 representations, that the tuple methods return the same values, and two ways to get None: df.astype({...: object}).where(df.notna(), None) and pd.set_option("future.infer_string", False) (pandas 2.1+). Both were checked against Athena (PandasCursor, unload=True, pandas 3.0.6): default str/nan; converted and opted-out results return (2, None, 'null_value') from fetchall(). The tests already assert the installed pandas representation.

Self-review of the repair (rounds 1 and 2; scope 9af4a07..e3aa3df, docs only): CLEAN. just docs lint and just docs build pass; the CSV rows of the table (NaN for both) and the binary NULL statements are unchanged and still hold on pandas 3. An independent follow-up is pending.

@laughingman7743 laughingman7743 Sep 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent follow-up (relayed) — reviewer: Codex CLI 0.156.0, model gpt-6-sol, codex exec --sandbox read-only, session 01a0d92e-4e60-71f0-adfb-788682133f60; static review of the repair 9af4a07..e3aa3df and the contracts it affects (PandasCursor CSV and Parquet read paths, fetch behavior, pandas cursor tests for pandas 2 and 3). Verdict: CLEAN — "The documented default NULL behavior matches the reviewed paths, and both documented ways to obtain None are consistent with them." No tests were run by the reviewer; the snapshot was unchanged afterward.

| `ArrowCursor` | CSV file | `''` | `''` | ❌ No |
| `ArrowCursor` + unload | Parquet file | `''` | `null` | ✅ Yes |
| `PolarsCursor` | CSV file | `''` | `null` | ✅ Yes |
Expand Down Expand Up @@ -177,11 +177,33 @@ df = cursor.execute("""
print(df)
# id value description
# 0 1 empty_string <- Empty string preserved
# 1 2 None null_value <- NULL is None
# 1 2 NaN null_value <- NULL is NaN (None with pandas 2)
# 2 3 hello normal_string

print(df['value'].isna().tolist())
# [False, True, False] <- Only NULL is NaN, empty string is not
# [False, True, False] <- Only NULL is missing, empty string is not
```

String columns follow the installed pandas version.
pandas 3 infers its `str` dtype and represents NULL as `NaN`; pandas 2 uses `object` columns and `None`.
`fetchone()`, `fetchmany()`, and `fetchall()` return the same values as the DataFrame.
Use `isna()` or `pandas.isna()` to detect NULL regardless of the pandas version.

To get `None` for NULL strings with pandas 3, convert the columns after reading:

```python
df = cursor.execute("SELECT ...").as_pandas()
df = df.astype({"value": object}).where(df.notna(), None)
```

Alternatively, turn off the pandas 3 string dtype for the whole process before executing queries.
String columns then use `object` with `None` for NULL, including rows returned by `fetchone()`, `fetchmany()`, and `fetchall()`.
The `future.infer_string` option exists in pandas 2.1 and later.

```python
import pandas as pd

pd.set_option("future.infer_string", False)
```

## ArrowCursor Behavior
Expand Down
1 change: 0 additions & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,6 @@ ignore = [
]

[tool.mypy]
python_version = "3.10"

@laughingman7743 laughingman7743 Sep 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review round 2 (claims, compatibility, operations) — CLEAN.

Claims checked: numpy 2.5.3 is resolved only for Python >= 3.12 (uv.lock markers), and its numpy/__init__.pyi:737 uses a type statement that mypy 1.14.0 and 2.3.1 both reject with python_version = "3.10"; a follow_imports = "skip" override for numpy did not avoid it because pandas imports numpy. Each tox pyathena environment runs just test pyathena, whose lint dependency runs mypy on that environment's interpreter, so the 3.10 CI job still type-checks for 3.10.

Operational consequence: a local just lint on Python 3.13 now checks 3.13 semantics only; 3.10-only incompatibilities surface in the 3.10 CI job. The AWS integration suites on all versions are running in CI and remain required before Ready.

follow_imports = "silent"
disallow_any_generics = true
strict_optional = true
Expand Down
17 changes: 10 additions & 7 deletions tests/pyathena/pandas/test_async_cursor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@
from tests import ENV
from tests.pyathena.conftest import connect

# pandas 3 infers its "str" dtype for strings, which represents NULL as NaN; pandas 2 uses
# object columns with None.
STRING_TYPE = pd.Series(["a"]).dtype.type
STRING_NULL = pd.Series(["a", None]).iloc[1]


class TestAsyncPandasCursor:
def test_binary_null_vs_empty(self, async_pandas_cursor):
Expand Down Expand Up @@ -587,7 +592,7 @@ def test_empty_and_null_string(self, async_pandas_cursor, parquet_engine):
# NULL and empty characters are correctly converted when the UNLOAD option is enabled.
np.testing.assert_equal(
result_set.fetchall(),
[("", "a"), ("N/A", "a"), ("NULL", "a"), (None, "a")],
[("", "a"), ("N/A", "a"), ("NULL", "a"), (STRING_NULL, "a")],
)
else:
np.testing.assert_equal(
Expand All @@ -598,12 +603,10 @@ def test_empty_and_null_string(self, async_pandas_cursor, parquet_engine):
result_set = future.result()
if async_pandas_cursor._unload:
# NULL and empty characters are correctly converted when the UNLOAD option is enabled.
assert result_set.fetchall() == [
("", "a"),
("N/A", "a"),
("NULL", "a"),
(None, "a"),
]
np.testing.assert_equal(
result_set.fetchall(),
[("", "a"), ("N/A", "a"), ("NULL", "a"), (STRING_NULL, "a")],
)
else:
assert result_set.fetchall() == [
("", "a"),
Expand Down
31 changes: 17 additions & 14 deletions tests/pyathena/pandas/test_cursor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
from tests import ENV
from tests.pyathena.conftest import connect

# pandas 3 infers its "str" dtype for strings, which represents NULL as NaN; pandas 2 uses
# object columns with None.
STRING_TYPE = pd.Series(["a"]).dtype.type

@laughingman7743 laughingman7743 Sep 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review of repair 9af4a07 (rounds 1 and 2, repair scope: git range-diff b2f0a8c → 9af4a07, test files only) — CLEAN.

Round 1 (behavior): dtype assertions stay exact; only the expected string type and NULL value come from the installed pandas. On pandas 2.3.3 the constants evaluate to numpy.object_ and None, identical to the previous hard-coded values; on pandas 3.0.6 they are str and nan. The UNLOAD NULL comparisons use np.testing.assert_equal, which treats NaN as equal, and still distinguish "" from NULL. No expectation became looser.

Round 2 (claims): the PR body's statements were checked against runs: CSV col_string, col_varchar, and unconverted col_array/col_map/col_struct text are str on pandas 3 (local Python 3.13 AWS run: the 3 test_complex_as_pandas cases pass after including indexes 13/15/17); UNLOAD NULL strings are NaN (4 null-string cases pass). The other 1743 pyathena tests and all sqla/sqla-async jobs passed in the first CI run. CI on 9af4a07 is still required before Ready.

STRING_NULL = pd.Series(["a", None]).iloc[1]


class TestPandasCursor:
@pytest.mark.parametrize(
Expand Down Expand Up @@ -641,17 +646,17 @@ def test_complex_as_pandas(self, pandas_cursor, chunksize):
np.int64,
np.float64,
np.float64,
np.object_,
np.object_,
STRING_TYPE,
STRING_TYPE,
np.datetime64,
np.object_,
np.datetime64,
np.object_,
STRING_TYPE,
np.object_,
STRING_TYPE,
np.object_,
np.object_,
np.object_,
np.object_,
STRING_TYPE,
np.object_,
)
rows = [
Expand Down Expand Up @@ -763,8 +768,8 @@ def test_complex_unload_as_pandas_pyarrow(self, pandas_cursor, parquet_engine):
np.int64,
np.float32,
np.float64,
np.object_,
np.object_,
STRING_TYPE,
STRING_TYPE,
np.datetime64,
np.object_,
np.object_,
Expand Down Expand Up @@ -1198,7 +1203,7 @@ def test_null_vs_empty_string(self, pandas_cursor, parquet_engine):
# NULL and empty characters are correctly converted when the UNLOAD option is enabled.
np.testing.assert_equal(
pandas_cursor.fetchall(),
[("", "a"), ("N/A", "a"), ("NULL", "a"), (None, "a")],
[("", "a"), ("N/A", "a"), ("NULL", "a"), (STRING_NULL, "a")],
)
else:
np.testing.assert_equal(
Expand All @@ -1208,12 +1213,10 @@ def test_null_vs_empty_string(self, pandas_cursor, parquet_engine):
pandas_cursor.execute(query, na_values=None, engine=parquet_engine)
if pandas_cursor._unload:
# NULL and empty characters are correctly converted when the UNLOAD option is enabled.
assert pandas_cursor.fetchall() == [
("", "a"),
("N/A", "a"),
("NULL", "a"),
(None, "a"),
]
np.testing.assert_equal(
pandas_cursor.fetchall(),
[("", "a"), ("N/A", "a"), ("NULL", "a"), (STRING_NULL, "a")],
)
else:
assert pandas_cursor.fetchall() == [
("", "a"),
Expand Down
Loading
Loading