-
-
Notifications
You must be signed in to change notification settings - Fork 768
⬆️ Add support for Python 3.13 #1289
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.
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
Conversation
I'll fix the test suite first in a separate PR. [UPDATE: done] |
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test suite for Python 3.13 is currently failing because pydantic-core==2.18.4
can't get installed with PyO3 0.21.2 on Python 3.13. We need PyO3 v0.22.0 or higher as that version supports Python 3.13, but I'm not sure yet why it's not pulling in the latest version - probably some other dependency is imposing an upper bound.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Ok, some progress: [UPDATE]: these linting errors appear because we're upgrading to Pydantic 2.10.6 (they don't appear when using Pydantic 2.9.2 for instance) |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Whoa, this was a lot of work! 😱 If dropping support for Python 3.7 would have made this easier, I would have definitely accepted it, just so you know you don't have to battle it so hard. 😅 If 3.7 is being problematic again in the future, let me know and we can just drop support for it. Maybe we can just do it soon, preemptively, it's already too old. I wanted to include any easy bug fixes in and make a final 3.7 release before dropping support, but if there's no single bug fix release to make, we can just drop it. We should also do it soon for 3.8 as well, we'll get into the same issues soon. 😬 About
|
Hehe, gotcha. The most time was spent on actually figuring out WHY But yes - I agree we could drop 3.7 (and maybe even 3.8) to avoid these kind of issues in the future.
Yep - exactly - that's the issue.
Yep - that's my understanding as well.
I see what you mean with respect to having (potentially future) access to better autocompletion & type hints when we keep the redefinition to the dictionary containing |
📝 Docs preview for commit 5b1f617 at: https://1205cfb7.sqlmodel.pages.dev |
Yep, I think so, we expect people to use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thank you for all the work put into this! 🚀
This ended up needing more changes than I had expected:
typing-extensions
. This then allows us to pull in Pydantic 2.8+ which supports Python 3.13.fastapi
imposes "only"pydanctic<3.0.0
), we would currently pull in Pydantic v.2.10IncEx
has changed, which means we need to change it as well or we'll getmypy
errors complaining about violating the Liskov substitution principleSQLModelMetaclass
, when we redefinemodel_fields
, we'd get thismypy
error:But I don't think we really need to redefine it? We'll be able to pass in asqlmodel.main.FieldInfo
object whereever apydantic.fields.FieldInfo
is expected, and the rest of the code base / tests / type checks don't seem to complain when we remove the redefinition (L482).As discussed below with Tiangolo, this PR now just adds an
ignore
statement for this.