Skip to content

⬆️ 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

Merged
merged 10 commits into from
Mar 6, 2025
Merged

Conversation

svlandeg
Copy link
Member

@svlandeg svlandeg commented Feb 7, 2025

This ended up needing more changes than I had expected:

  • To allow a valid resolution of dependencies, for Python 3.13 we need to allow a higher version of typing-extensions. This then allows us to pull in Pydantic 2.8+ which supports Python 3.13.
  • If we keep the upper version of Pydantic unbounded (fastapi imposes "only" pydanctic<3.0.0), we would currently pull in Pydantic v.2.10
  • Since Pydantic 2.10.0, the definition of IncEx has changed, which means we need to change it as well or we'll get mypy errors complaining about violating the Liskov substitution principle
  • In SQLModelMetaclass, when we redefine model_fields, we'd get this mypy error:
sqlmodel\main.py:482: error: Incompatible types in assignment (expression has type "dict[str, sqlmodel.main.FieldInfo]", base class "ModelMetaclass" defined the type as "dict[str, pydantic.fields.FieldInfo]")  [assignment]

But I don't think we really need to redefine it? We'll be able to pass in a sqlmodel.main.FieldInfo object whereever a pydantic.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.

@svlandeg svlandeg marked this pull request as draft February 7, 2025 10:08
@svlandeg
Copy link
Member Author

svlandeg commented Feb 7, 2025

I'll fix the test suite first in a separate PR.

[UPDATE: done]

@svlandeg svlandeg self-assigned this Feb 7, 2025
Copy link

📝 Docs preview for commit 4891994 at: https://ecc6a564.sqlmodel.pages.dev

Copy link
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.

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.

Copy link

github-actions bot commented Mar 3, 2025

📝 Docs preview for commit 73616d3 at: https://1cf4625d.sqlmodel.pages.dev

Copy link

github-actions bot commented Mar 4, 2025

📝 Docs preview for commit 836f367 at: https://8b53db85.sqlmodel.pages.dev

Copy link

github-actions bot commented Mar 4, 2025

📝 Docs preview for commit 77d8445 at: https://82881eb9.sqlmodel.pages.dev

Copy link

github-actions bot commented Mar 4, 2025

📝 Docs preview for commit 3134d78 at: https://631516ea.sqlmodel.pages.dev

@svlandeg
Copy link
Member Author

svlandeg commented Mar 4, 2025

Ok, some progress: sqlmodel's pin on typing-extensions was preventing us to use the latest pydantic version compatible with Python 3.13. If we relax that pin, we can get correct Pydantic versions for Python 3.13. The CI is then still failing with a linting error, which I'll look into now.

[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)

Copy link

github-actions bot commented Mar 4, 2025

📝 Docs preview for commit 03d348e at: https://e454acf8.sqlmodel.pages.dev

Copy link

github-actions bot commented Mar 4, 2025

📝 Docs preview for commit 14ee1a8 at: https://f4fa81c8.sqlmodel.pages.dev

Copy link

github-actions bot commented Mar 4, 2025

📝 Docs preview for commit 6a96dba at: https://ba34d88f.sqlmodel.pages.dev

@svlandeg svlandeg removed their assignment Mar 4, 2025
@svlandeg svlandeg marked this pull request as ready for review March 4, 2025 09:57
@tiangolo
Copy link
Member

tiangolo commented Mar 4, 2025

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 model_fields

I'm pretty sure the problem is that Pydantic has it defined as a dict[str, pydantic.fields.FieldInfo] and dict doesn't support subclasses of the parameters defined (the erudite term is "invariant" I think 😅, dict is invariant). I suspect they don't really need it to be a dict, it could be any mapping, so Mapping[str, pydantic.fields.FieldInfo] would have probably worked as well, and Mapping allows subclasses in the value types (erudite term: Mapping is covariant in the second parameter, if I'm not wrong).

It's also not common for people to extend that in Pydantic, it's not a fully public object (it's not documented), so I don't judge it and wouldn't necessarily expect them to change it. 😅

Nevertheless, our code should be okay, so we could add a # type: ignore there and it would be fine.

Now, what do we get from our definition?

Whenever someone tries to iterate on the model fields of a model, they would get autocompletion and inline errors for our custom extra attributes. E.g.:

for f in Hero.model_fields.values():
    print(f.index)

In this case, they would get autocompletion for f.index.

...but, checking that code, I also realize that it is not properly typed, those extra attributes have no types, so they show as existing, but as Any. So, the "advantage" we could get is not even properly/fully implemented yet.

I would say we can add a type ignore and later in another PR add the types, just to save the autocompletion there, not sure how useful and used it is, but maybe.

What do you think?

@svlandeg
Copy link
Member Author

svlandeg commented Mar 4, 2025

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. 😅

Hehe, gotcha. The most time was spent on actually figuring out WHY uv's dependency resolution tracked back to such an old version of Pydantic. Once I found out that the typing-extensions pin was the culprit, the fix was actually an easy one.

But yes - I agree we could drop 3.7 (and maybe even 3.8) to avoid these kind of issues in the future.

About model_fields

I'm pretty sure the problem is that Pydantic has it defined as a dict[str, pydantic.fields.FieldInfo] and dict doesn't support subclasses of the parameters defined (the erudite term is "invariant" I think 😅, dict is invariant).

Yep - exactly - that's the issue.

I suspect they don't really need it to be a dict, it could be any mapping, so Mapping[str, pydantic.fields.FieldInfo] would have probably worked as well, and Mapping allows subclasses in the value types (erudite term: Mapping is covariant in the second parameter, if I'm not wrong).

Yep - that's my understanding as well.

I would say we can add a type ignore and later in another PR add the types, just to save the autocompletion there, not sure how useful and used it is, but maybe.

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 sqlmodel's FieldInfo objects. I assume we would never really expect to add Pydantic's FieldInfo objects into SQLModelMetaclass, right? In that case I agree that we can just add the ignore. I'll push that change.

Copy link

github-actions bot commented Mar 4, 2025

📝 Docs preview for commit 5b1f617 at: https://1205cfb7.sqlmodel.pages.dev

@tiangolo
Copy link
Member

tiangolo commented Mar 6, 2025

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 sqlmodel's FieldInfo objects. I assume we would never really expect to add Pydantic's FieldInfo objects into SQLModelMetaclass, right? In that case I agree that we can just add the ignore. I'll push that change.

Yep, I think so, we expect people to use sqlmodel.Field instead of pydantic.Field, so we would always have our own custom FieldInfo there. So, yep, I think that makes sense. 🤓

Copy link
Member

@tiangolo tiangolo left a 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! 🚀

@tiangolo tiangolo merged commit b1349da into fastapi:main Mar 6, 2025
28 checks passed
@svlandeg svlandeg deleted the update/python branch March 6, 2025 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants