Skip to content

Remove get_double_type from the SQLAlchemy types module - #860

Merged
laughingman7743 merged 1 commit into
masterfrom
feat/remove-get-double-type
Sep 27, 2026
Merged

laughingman7743 merged 1 commit into
masterfrom
feat/remove-get-double-type

Conversation

@laughingman7743

Copy link
Copy Markdown
Member

WHAT

  • Remove get_double_type() from pyathena.sqlalchemy.types, and from its __all__.
  • Remove the now-unused typing.Any import.
  • Replace test_get_double_type with test_double_column_type. It keeps the assertion that reflected double columns map to sqlalchemy.types.DOUBLE (ischema_names["double"]).

WHY

Follow-up to #845 / #858, requested by the maintainer for the 4.0.0 release. This change will not be backported to 3.x.

Since #858 requires SQLAlchemy 2.0, get_double_type() only returned types.DOUBLE, and nothing in PyAthena calls it.
GitHub code search found no use outside this repository. The search covers only public repositories.

Release note (breaking): pyathena.sqlalchemy.types.get_double_type is removed. Use sqlalchemy.types.DOUBLE.

TEST

Tested commit: 20ed3cf.

  • just lint: passed, including license headers.
    tests/pyathena/sqlalchemy/test_types.py stays in the existing header exclusion list in scripts/config/license_headers.toml.
  • uv run pytest tests/pyathena/sqlalchemy/test_types.py tests/pyathena/sqlalchemy/test_compiler.py tests/pyathena/sqlalchemy/test_array.py -k "not Integration": 337 passed.
  • git grep get_double_type: no references remain.

Not run locally: AWS integration suites (left to CI). The change removes an unused helper and touches no query path.

🤖 Generated with Claude Code

With SQLAlchemy 2.0 required (#845), get_double_type only returned
types.DOUBLE and nothing in PyAthena calls it. Remove it from
pyathena.sqlalchemy.types for the 4.0.0 release; use
sqlalchemy.types.DOUBLE instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
from pyathena.sqlalchemy.base import ischema_names

assert get_double_type() is types.DOUBLE
def test_double_column_type():

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, rounds one and two (Claude Code)

Scope: git diff 0ba0875c72c702e0b893843d3c50aa98b4a61b8f..20ed3cf385b924e20e9e81982b7cc83ab7b35f2b, covering pyathena/sqlalchemy/types.py and tests/pyathena/sqlalchemy/test_types.py, plus the PR body and commit message.

Round one — behavior: CLEAN.

  • get_double_type had no callers left in pyathena/, tests/, docs/, or benchmarks/ (git grep).
  • ischema_names["double"] is still types.DOUBLE, and the replacement test keeps that assertion.
  • typing.Any had no other use in types.py.
  • Removing the entry from __all__ also drops it from from pyathena.sqlalchemy.types import *.
  • Validation: just lint passed (including license headers); 337 no-AWS tests passed.

Round two — claims and compatibility: CLEAN.

  • This is a breaking removal of a public name, which the PR body scopes to 4.0.0 and flags for the release notes, with sqlalchemy.types.DOUBLE as the replacement.
  • The "no outside use" claim is limited to what GitHub code search can see (public repositories), and the body says so.
  • No generated API docs list the function; docs/ has no autodoc directive for it.

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.

CI note (Claude Code)

Run 36288246126 at 20ed3cf, attempt 1: test-sqla / run (3.10) reported 589 passed and 4 errors.

  • The errors were all in the setup of InsertBehaviorTest (test_autoclose_on_insert, test_autoclose_on_insert_implicit_returning, test_empty_insert, test_empty_insert_multiple), which share one fixture table.
  • Athena failed the fixture's Iceberg CREATE TABLE manual_pk with AmazonS3Exception: Service Unavailable (Status Code: 503). This is a transient server-side S3 error, unrelated to removing an unused helper.
  • Attempt 2 of that job passed. All 19 checks now pass; the 3 skipped entries belong to the run from the Draft period.

@@ -32,19 +32,9 @@
"AthenaStruct",
"AthenaTimestamp",
"Tinyint",

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)

This is a static review of a detached snapshot at the head SHA; the reviewer ran nothing.

  • Reviewer: Codex CLI 0.157.0, model gpt-6-sol, session 01a0e0a9-0db2-7402-bf14-8c02ef32bd4b.
  • Invocation: codex exec -s read-only --ephemeral.
  • Scope: 0ba0875c72c702e0b893843d3c50aa98b4a61b8f..20ed3cf385b924e20e9e81982b7cc83ab7b35f2b.
  • The prompt contained the literal diff and constraints only.

Covered surfaces:

  • The two changed files.
  • References across code, tests, docs (including docs/api/*.rst), benchmarks, and scripts.
  • The DOUBLE reflection path, imports, and the license-header configuration.

Result: CLEAN.

  • No get_double_type references remain.
  • The replacement test checks the mapping used by reflection.
  • The test file's missing header is covered by the existing exemption.

The snapshot and PR worktree were unchanged afterwards.

@laughingman7743
laughingman7743 marked this pull request as ready for review September 27, 2026 02:20
@laughingman7743
laughingman7743 merged commit 5311856 into master Sep 27, 2026
40 of 41 checks passed
@laughingman7743
laughingman7743 deleted the feat/remove-get-double-type branch September 27, 2026 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant