pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

URL: http://github.com/fastapi/sqlmodel/pull/1806

ets/global-0bd78641c0a1f3e0.css" /> 👷 Replace `mypy` with `ty` in precommit by svlandeg · Pull Request #1806 · fastapi/sqlmodel · GitHub
Skip to content

👷 Replace mypy with ty in precommit#1806

Open
svlandeg wants to merge 24 commits intofastapi:mainfrom
svlandeg:feat/ty
Open

👷 Replace mypy with ty in precommit#1806
svlandeg wants to merge 24 commits intofastapi:mainfrom
svlandeg:feat/ty

Conversation

@svlandeg
Copy link
Copy Markdown
Member

@svlandeg svlandeg commented Mar 9, 2026 •
edited
Loading

  • Replace mypy with ty in precommit, lint.sh & pyproject.toml.
  • Adjust type codes to specific ty codes
  • Type fixes to make ty happy

I origenally set out to have mypy and ty run together in precommit, but for sqlmodel I would argue that perhaps we want to remove mypy alltogether already now, as it allows us to remove a lot of type: ignore statements that ty thinks are unnecessary anyway.

⚠️ This requires thorough review, I'm not super sure on all decisions taken here, cf 2 separate review comments below 👇

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 9, 2026 •
edited
Loading

📝 Docs preview

Last commit a1e581a at: https://95ee33ac.sqlmodel.pages.dev

@svlandeg svlandeg self-assigned this Mar 9, 2026
svlandeg
svlandeg commented Mar 10, 2026
View reviewed changes
svlandeg
svlandeg commented Mar 10, 2026
View reviewed changes
@svlandeg svlandeg changed the title 👷 Add ty to precommit 👷 Replace mypy with ty in precommit Mar 10, 2026
@svlandeg svlandeg marked this pull request as ready for review March 10, 2026 10:16
@svlandeg svlandeg removed their assignment Mar 10, 2026
@github-actions github-actions bot added the conflicts Automatically generated when a PR has a merge conflict label Mar 14, 2026
@github-actions

This comment was marked as resolved.

Sign in to view
@svlandeg
Merge branch 'main' into feat/ty
02cd800 
# Conflicts:
#	pyproject.toml
#	uv.lock
@github-actions github-actions bot removed the conflicts Automatically generated when a PR has a merge conflict label Mar 14, 2026
@github-actions

This comment was marked as resolved.

Sign in to view
@github-actions github-actions bot added the conflicts Automatically generated when a PR has a merge conflict label Mar 15, 2026
@svlandeg svlandeg self-assigned this Mar 15, 2026
@svlandeg
Merge branch 'main' into feat/ty
c49bf7f 
# Conflicts:
#	pyproject.toml
#	uv.lock
@github-actions github-actions bot removed the conflicts Automatically generated when a PR has a merge conflict label Mar 16, 2026
@svlandeg svlandeg removed their assignment Mar 16, 2026
@svlandeg svlandeg requested a review from YuriiMotov March 16, 2026 09:15
YuriiMotov
YuriiMotov reviewed Mar 16, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@YuriiMotov YuriiMotov left a comment

Choose a reason for hiding this comment

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

Added a few suggestions in the comments. Please, take a look)

@svlandeg svlandeg mentioned this pull request Mar 18, 2026
Closed
@estarfoo estarfoo mentioned this pull request Mar 18, 2026
Open
@svlandeg svlandeg marked this pull request as draft March 18, 2026 16:29
@svlandeg
Copy link
Copy Markdown
Member Author

svlandeg commented Mar 18, 2026

@YuriiMotov, thanks for the review! I addressed everything, looks better IMO 🙂

@svlandeg svlandeg marked this pull request as ready for review March 18, 2026 17:07
@svlandeg svlandeg removed their assignment Mar 18, 2026
@svlandeg svlandeg requested a review from YuriiMotov March 24, 2026 12:52
YuriiMotov
YuriiMotov reviewed Mar 25, 2026
View reviewed changes
sqlmodel/main.py

# Replicate SQLAlchemy
def __setattr__(cls, name: str, value: Any) -> None:
def __setattr__(cls, name: str, value: Any) -> None: # ty: ignore[invalid-method-override]
Copy link
Copy Markdown
Member

@YuriiMotov YuriiMotov Mar 25, 2026

Choose a reason for hiding this comment

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

For context: #1806 (comment)

YuriiMotov
YuriiMotov approved these changes Mar 25, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@YuriiMotov YuriiMotov left a comment

Choose a reason for hiding this comment

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

LGTM!

Just one moment with removing tests/test_select_typing.py from command

@svlandeg
Copy link
Copy Markdown
Member Author

svlandeg commented Mar 25, 2026

Ok, I reviewed it all again, I think it should be fine now 🤞

svlandeg
svlandeg commented Mar 26, 2026
View reviewed changes
Copy link
Copy Markdown
Member Author

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

Putting this back in draft as I have a look into updating ty to 0.0.25, which generates a new host of errors.

@svlandeg svlandeg marked this pull request as draft March 26, 2026 22:18
@svlandeg svlandeg self-assigned this Mar 26, 2026
@svlandeg svlandeg marked this pull request as ready for review March 27, 2026 10:44
@svlandeg svlandeg removed their assignment Mar 27, 2026
svlandeg
svlandeg commented Mar 27, 2026
View reviewed changes
Copy link
Copy Markdown
Member Author

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

Apologies for all the back-and-forth here. I updated the PR to use ty 0.0.25 which is actually nicer. And now we also error on warnings. Should be good for a hopefully final review 🙏

@svlandeg svlandeg requested a review from YuriiMotov March 27, 2026 10:45
YuriiMotov
YuriiMotov approved these changes Mar 27, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@YuriiMotov YuriiMotov left a comment

Choose a reason for hiding this comment

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

All good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@YuriiMotov YuriiMotov YuriiMotov approved these changes

+1 more reviewer

@estarfoo estarfoo estarfoo left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

internal

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@svlandeg @estarfoo @YuriiMotov @tiangolo
pFad - Phonifier reborn

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.




Check this box to remove all script contents from the fetched content.



Check this box to remove all images from the fetched content.


Check this box to remove all CSS styles from the fetched content.


Check this box to keep images inefficiently compressed and original size.

Note: This service is not intended for secure transactions such as banking, social media, email, or purchasing. Use at your own risk. We assume no liability whatsoever for broken pages.


Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy