-
Notifications
You must be signed in to change notification settings - Fork 17
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
Update deprecated pydantic model Config class to model_config attribute #103
base: master
Are you sure you want to change the base?
Conversation
@spwoodcock thanks but please open a forum ticket to discuss proposed changes before launching in to a PR - you acknowledged this process in March this year. It is complicated further by opening 2x more PRs based on this one. I would also ask that you run Is your goal to silence this particular deprecation warning? Upgrading pydantic to be ready for v3 will require more changes - I can think of at least the use of pydantic.v1 validators. The migration guide is rather long so there's probably more, I'm hoping that their |
@lindsay-stevens yes you are right, I saw what I wrote previously, my bad entirely 😓 I'm just so used to this being the typical workflow for any other project I contribute to and got carried away. Anyway, I was doing this work regardless & I felt in the right headspace to just get it done now. It probably wouldn't have been done had I left it a few days, as something else would probably require my attention. Please feel free to close, modify, merge, whatever! 😄 |
Regarding you other points:
The instructions to But regardless, the commits in a PR are a bit irrelevant if you do a squash merge no?
You are completely right, a migration to Pydantic v3 would take more work. As you say, I saw the v1 validators, but didn't want to dive into that now. Pydantic v3 is not released and as far as I know not on the near horizon. Is there any downside to fixing deprecations from v1 --> v2, considering we are using v2 now? |
This PR
Not to mention the static type checking complaints about overriding
Config
:model_config
attribute should be used.ConfigDict
in every file, I instead created aFrozenModel
class to be used:What has been done to verify that this works as intended?
Why is this the best possible solution? Were any other approaches considered?
Config
class will be removed in pydantic v3, making the upgrade of this lib more difficult.How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
No change.
Do we need any specific form for testing your changes? If so, please attach one.
Nope.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
Nope.
Before submitting this PR, please make sure you have:
tests
python -m unittest
and verified all tests passruff format pyodk tests
andruff check pyodk tests
to lint code