-
Notifications
You must be signed in to change notification settings - Fork 144
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
Remove pydantic upper bound #189
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
antonymilne
requested review from
Joseph-Perkins,
huong-li-nguyen,
maxschulz-COL and
lingyielia
as code owners
December 1, 2023 16:46
Signed-off-by: Antony Milne <49395058+antonymilne@users.noreply.github.com>
This reverts commit efc3169.
lingyielia
approved these changes
Dec 7, 2023
petar-qb
approved these changes
Dec 7, 2023
huong-li-nguyen
approved these changes
Dec 7, 2023
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.
LGTM 👍
l0uden
approved these changes
Dec 7, 2023
nadijagraca
approved these changes
Dec 7, 2023
9 tasks
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Currently we have
pydantic<2
, which is not ideal for compatibility with other tools. It's easy to remove this so long as we make sure to always do pydantic imports using the following sort of pattern:We are still using the pydantic v1 API everywhere and so nothing else changes. At some point in the future we should switch fully to v2 but that is a bigger project (and would make us incompatible with kedro-viz, which currently pins
pydantic<2
).The changes have been made across vizro-core and vizro-ai.
As a nice bonus I've been able to remove a lot of the pydantic-mypy
ignore
s.I've refactored the test jobs quite a bit and made a new one that tests the lower bound
pydantic==1.10.13
. All other test jobs will pip install the latest compatible requirements, which would now be pydantic 2.5.2. @l0uden has also added new tests to the qa repo to test the lower bound.So basically if you forget to do the imports correctly, tests should catch it so long as we have good test coverage 🤞
Checklist
Types of changes
Notice
I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":