-
Notifications
You must be signed in to change notification settings - Fork 14
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
Support Pydantic v2 #364
Conversation
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.
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.
Hey Zhihan, thanks for the feedback! |
Hey @Nour-Mws, what I did was:
and it doesn't complain anything. However, after rethinking a bit, I tried to add
so I'm able to reproduce issue #360 now. |
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.
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) 💯 🚀 💯
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:
pydantic
orpydantic.v1
depending on whether v1 or v2 are installed (respectively)pyproject.toml
as both v1 and v2mypy
by a couple of releases since it was failing withpydantic
v2# type: ignore[no-redef]
annotation as otherwise MyPy will complain about the same modules being "redefined" in the imports happening in thetry/except
clause.This also bumps the dev version of
pydantic
in the Poetry lock to2.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: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 atry/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:
Cannot find implementation or library stub for module named "pydantic.v1"
: this is to be expected as1.10.12
does not have a.v1
in its API.Unused "type: ignore" comment
: this is very much used when2.3.0
is installed as opposed to1.10.12
:)To recap: FawltyDeps now works with both Pydantic v1 and v2 but the CI only supports v2.