FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

馃悰 Fix `tuple_` return type annotation by kakeruzoku 路 Pull Request #1639 路 fastapi/sqlmodel 路 GitHub

  1. fastapi
  2. sqlmodel

Repository navigation

馃悰 Fix tuple_ return type annotation - #1639

Merged
svlandeg merged 4 commits into
fastapi:mainfrom
kakeruzoku:main
Mar 2, 2026
ConversationCommits聽(4)ChecksFiles changed
Merged

馃悰 Fix tuple_ return type annotation#1639
svlandeg merged 4 commits into
fastapi:mainfrom
kakeruzoku:main

Conversation

kakeruzoku commented Nov 7, 2025

Copy link
Copy Markdown
Contributor

Fixed an issue where sqlmodel.tuple_ returns <class 'sqlalchemy.sql.elements.Tuple'> but the type hint points to <class 'tuple'>.

The current workaround for this mistake, # type: ignore[return-value], has been removed.

sqlmodel/sqlmodel/sql/expression.py

Line 195 in 5c1b89a

) -> Tuple[Any, ...]:

YuriiMotov changed the title 馃悰 Fix tuple_ type hinting mistake 馃悰 Fix tuple_ return type annotation Nov 10, 2025
YuriiMotov added the bug Something isn't working label Nov 10, 2025
YuriiMotov approved these changes Nov 10, 2025
View reviewed changes

YuriiMotov left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM!

SQLAlchemy's tuple_ returns Tuple imported as from .elements import Tuple.

@kakeruzoku, thanks!

github-actions Bot added the conflicts Automatically generated when a PR has a merge conflict label Dec 26, 2025

This comment was marked as resolved.

Sign in to view

yashovardhan99 commented Jan 4, 2026 •
edited
Loading

Copy link
Copy Markdown

Any ETA on when this will be merged? (my type-checker just complained about this, and I have imported tuple_ from sqlalchemy directly until this is fixed).

Seems like a simple conflict due to the recent python version upgrade. I can also create a new PR for this change (with the conflict resolved) if the original author is not available.

github-actions Bot removed the conflicts Automatically generated when a PR has a merge conflict label Jan 4, 2026

kakeruzoku commented Jan 4, 2026

Copy link
Copy Markdown
Contributor Author

I fixed the conflict.

This comment was marked as resolved.

Sign in to view
github-actions Bot added the conflicts Automatically generated when a PR has a merge conflict label Feb 20, 2026
github-actions Bot removed the conflicts Automatically generated when a PR has a merge conflict label Feb 21, 2026

SamuNatsu commented Mar 2, 2026

Copy link
Copy Markdown

Any progress to be merged? I've encounter the type issue.

svlandeg approved these changes Mar 2, 2026
View reviewed changes

svlandeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks for the PR, looks good! Merging now 馃檹

svlandeg merged commit 2eb65df into fastapi:main Mar 2, 2026
23 checks passed
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

svlandeg svlandeg approved these changes
YuriiMotov YuriiMotov approved these changes

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL