-
Notifications
You must be signed in to change notification settings - Fork 104
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
Pydantic V2 API Migration #148
Pydantic V2 API Migration #148
Conversation
…re-build the dependency on pydantic
from pydantic.v1 import BaseModel, Extra, Field | ||
from pydantic.v1.class_validators import ROOT_VALIDATOR_CONFIG_KEY, root_validator | ||
from pydantic.v1.error_wrappers import ErrorWrapper, ValidationError | ||
from pydantic.v1.errors import ConfigError, PydanticValueError |
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.
These have been removed in pydantic v2 https://docs.pydantic.dev/latest/migration/#removed-in-pydantic-v2
I think you are on the right track here.
|
model_config = ConfigDict( | ||
extra="forbid", | ||
populate_by_name=True, | ||
validate_assignment=True, | ||
) |
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.
this replaces the class Config: ...
approach in pydantic v2
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.
@nazrulworld @bwalsh the previously supported json_loads
and json_dumps
config parameters were removed in pydantic v2
@nazrulworld @bwalsh in the Pydantic V2 migration guide
I think that the decision they made is that while a domain modeling library can provide convenience utilities for data IO, that is overreach into logic that ought to be fully owned by the user (or some other library). I think I agree with this and this thinking could be applied What do you think of removing the IO logic in An alternative would be copying the pydantic v1 IO utilities into Also, happy holidays :) |
I am fully agree with you about separation of IO functionalities. But for now we should keep those functionalities inside FHIRAbstractModel. |
Hey @Tshimanga @bwalsh @nazrulworld, just wanted to check in on this. What's the current status? We need to rework codegen and then it should work? Or are there still changes needed to the base models here? I'm trying to use this in a FastAPI app built on pydantic V2, but I currently can't use the models in the framework due to V1 usage. With a little direction on what's remaining I might be able to help with this effort. |
|
Hi @nazrulworld @bwalsh @Tshimanga I saw in the thread you already managed to fix the pydantic version compatibility for the package managers and alike. I would like to see this in action and am willing to give some cents of reviewing or contribute myself, yet right now this PR seems a bit stale. Kind regards |
@skaanbilly2 @nickderobertis Please feel free to contribute to getting this over of the finish line as I'm still a bit preoccupied. At the moment the moment the main effort has been migrating the |
@Tshimanga @skaanbilly2 I could manage some time on this development at coming sommer (around mid July or earlier). We can have a meeting about collective contributions, if you guys have time. |
|
Hi @Tshimanga |
@nazrulworld I definitely don't want to be a blocker for this. I'll be preoccupied for the next couple of weeks and then I'll be available to pick this up again. I'm happy to either share this branch or create a new one. If you've got spare time for this now then lets do whatever is most convenient for you |
@nazrulworld also definitely still interested in meeting to talk through the rest of the work :) |
@Tshimanga thanks a lot for the clarification. I think, I will try with my approach and observe the progress. After then we can combine both our ideas. |
Hi @Tshimanga @bwalsh and all What I did
What is current status
Your contributions are welcome
|
Here is the first beta release https://pypi.org/project/fhir.resources/8.0.0b1/ |
It looks like pydantic is quite thoroughly interleaved in
fhir.resources
. I haven't yet identified clear boundaries or individually migrate-able portions of the codebase. I think that we may have to collaborate commit by commit discussing how to navigate each breaking change.Hopefully we can find a way to better encapsulate some of the fancy pydantic usage as we go.
I don't mind taking point on making sure this work/pr keeps moving forward, but I expect I'll need frequent input from @nazrulworld along the way.