Skip to content
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

Support Pydantic v2 #364

Merged
merged 4 commits into from
Sep 12, 2023
Merged

Support Pydantic v2 #364

merged 4 commits into from
Sep 12, 2023

Conversation

Nour-Mws
Copy link
Collaborator

@Nour-Mws Nour-Mws commented Sep 6, 2023

Closes #360.
We have so far been using Pydantic v1.
Pydantic v2 comes with a revisited API that is incompatible with the one from v1.
Since we'd like projects that include FawltyDeps in their dependencies not to have their own dependency versions clash with those of FawltyDeps (see the case in #360), it makes sense for us to support both v1 and v2 of Pydantic. This is made possible by the v1 functionality still being usable with import pydantic.v1 as pydantic.
This PR:

  • Conditionally imports pydantic or pydantic.v1 depending on whether v1 or v2 are installed (respectively)
  • Defines the supported versions of pydantic in pyproject.toml as both v1 and v2
  • Bumps the minimal version of mypy by a couple of releases since it was failing with pydantic v2
  • Adds a # type: ignore[no-redef] annotation as otherwise MyPy will complain about the same modules being "redefined" in the imports happening in the try/except clause.

This also bumps the dev version of pydantic in the Poetry lock to 2.3.0 (the current version).
Note that I locally ran the CI with both 1.10.14 (previously in the lock) and v 2.3.0 with the changes in this PR:

  • the tests pass for both versions
  • MyPy passes for v2
  • MyPy will complain for v1

Since the functionality is preserved for both pydantic v1 and v2 (as evidenced by the tests passing), and the CI passes with flying colors with v2, I opted not to spend any more time trying to coerce MyPy to also work with v1. This is fruitless as the typechecking will only be running with the dev version anyway, which is v2.

Also note that the issue with MyPy being a pain to work with when you import in a try/except (with or without libraries having different APIs) is a known one. See for example this SO question and python/mypy#8823. There is specific support for if/else clauses with Python or system versions but this is not the case here.

For a somewhat related discussion, see pydantic/pydantic#6022.

This is an excerpt of the errors when running MyPy with v1.10.12 of Pydantic:

fawltydeps/settings.py:12: error: Cannot find implementation or library stub for module named "pydantic.v1"  [import]
fawltydeps/settings.py:13: error: Cannot find implementation or library stub for module named "pydantic.v1.env_settings"  [import]
fawltydeps/settings.py:13: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
fawltydeps/settings.py:14: error: Cannot find implementation or library stub for module named "pydantic.v1.json"  [import]
fawltydeps/settings.py:16: error: Unused "type: ignore" comment
fawltydeps/settings.py:17: error: Unused "type: ignore" comment
fawltydeps/settings.py:18: error: Unused "type: ignore" comment
fawltydeps/settings.py:43: error: Argument 2 to "__call__" becomes "Any" due to an unfollowed import  [no-any-unimported]
fawltydeps/settings.py:101: error: Class cannot subclass "BaseSettings" (has type "Any")  [misc]

  • Cannot find implementation or library stub for module named "pydantic.v1": this is to be expected as 1.10.12 does not have a .v1 in its API.
  • Unused "type: ignore" comment: this is very much used when 2.3.0 is installed as opposed to 1.10.12 :)

To recap: FawltyDeps now works with both Pydantic v1 and v2 but the CI only supports v2.

Copy link
Collaborator

@zz1874 zz1874 left a comment

Choose a reason for hiding this comment

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

The code looks good to me!
I've also tried it locally and it works well 👍 Linter doesn't complain with the proper comment # type: ignore[...].

However, I'm not able to reproduce the related issue, (I've tried to run our previous fawltydeps in a fake project using pydantic v2, but it also doesn't complain), so I won't approve this PR now and will leave the discussion open.

@Nour-Mws
Copy link
Collaborator Author

Nour-Mws commented Sep 8, 2023

Hey Zhihan, thanks for the feedback!
I think I don't follow your second point. Could you please share the setting you tested?

@zz1874
Copy link
Collaborator

zz1874 commented Sep 11, 2023

Hey @Nour-Mws, what I did was:

  • creating a fake project that uses pydantic v2
  • running fawltydeps (the version before this PR) under the project

and it doesn't complain anything.

However, after rethinking a bit, I tried to add pydantic v2 and fawltydeps as two dependencies in the fake project, and this time it says

$ poetry add fawltydeps
Using version ^0.13.0 for fawltydeps

Updating dependencies
Resolving dependencies... (0.1s)

Because no versions of fawltydeps match >0.13.0,<0.14.0
 and fawltydeps (0.13.0) depends on pydantic (>=1.10.4,<2.0.0), fawltydeps (>=0.13.0,<0.14.0) requires pydantic (>=1.10.4,<2.0.0).
So, because fake-pydantic-v2 depends on both pydantic (2.3.0) and fawltydeps (^0.13.0), version solving failed.

so I'm able to reproduce issue #360 now.

Copy link
Member

@jherland jherland left a comment

Choose a reason for hiding this comment

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

Love the PR description. Fully agree that the type checking problems with Pydantic v1 installed can be ignored, especially when the test suite is all green.

The changes themselves are straightforward and unsurprising (given the thorough PR description) 💯 🚀 💯

@Nour-Mws Nour-Mws merged commit d2c8204 into main Sep 12, 2023
33 checks passed
@Nour-Mws Nour-Mws deleted the nour/support-pydantic-v2 branch September 12, 2023 15:19
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.

Support pydantic v2
3 participants