-
Notifications
You must be signed in to change notification settings - Fork 10
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 to supporting Pydantic V2 #203
Conversation
Note: bump-pydantic is still in beta
Replace its use with `pydantic.BaseModel.model_dump(mode='json', exclude_none=True)`
Its use can be replaced with `pydantic.BaseModel.model_construct`
Even if for "practice" I think it might good to provide a proper deprecation cycle for those happen there is a tool using dandischema directly and not using any "pydantic specifics": please keep those in API, but make them raise |
@satra I think @yarikoptic's has a valid point. May be we should keep those APIs in place for a while instead of removing them now, as you said in Monday's meeting. I can keep those API in place by wrapping the new features provided by Pydantic 2.0. |
@candleindark - i'm fine with a deprecation cycle. however, if nobody is using them, i'm also ok with the decision to remove. we will be doing a significant change anyway with pydantic 2. if there are significant changes made to other pieces (cli, server), then let's remove those functions. if not, we can put them into deprecated mode. |
But mark it as deprecated
But mark it as deprecated
50a794c
to
5528c3d
Compare
Specifying the most compact separators to use in JSON serialization, `(",", ":")`, is no longer needed for they are the default separators to use for JSON serialization in Pydantic V2
This is the implicit default in Pydantic V1
…`BaseModel.model_json_schema`
…ave incompatible changes)
for more information, see https://pre-commit.ci
current state: I boosted schema version to 0.7.0 and we need to review if any upgrades to the |
The logic of the result is equivalent to the one before the modification
@candleindark - could you please address the linting error? @yarikoptic - i'm assuming we are releasing without the CLI changes? shall we release once the linting error is fixed? |
@satra No, we can't release yet, as the TODO item in this commit is still undone. |
there hasn't been any data model changes, has there? and i'm assuming the migrate tests all pass? the only time a migration code change is required is if we change the data model. |
I've converted this PR to a draft in order to prevent accidental merging until we can confirm and are sure that all necessary changes have been made. |
There is no change in the data model because of the Pydantic upgrade to V2
For dandi JSON schema 0.7.0 or above
9c9085d
to
b718dc2
Compare
So that mypy doesn't complain
0.6.5 is more sensible since the changes in the schema are not breaking changes, and there is no change in the models and the JSON serialization of these models
The test failed. It seems to be a github problem, but I don't have the option to re-run it. |
Did someone just re-ran the test for me. It just re-ran and succeeded. |
I resolved merge conflict - trivial conflict in imports |
Pydantic 2.0's JSON schema export no longer specifies a `type` field for the `schemaKey` property, and leaves it undefined instead. This change makes it so an undefined type also delegates it as a "basic schema", meaning it will not be rendered as its own tab in the meditor.
This PR migrates Pydantic from V1 to V2. It closes #176.
This PR deprecates the following APIs:
dandischema.models.DandiBaseModel.json_dict
pydantic.BaseModel.model_dump(mode='json', exclude_none=True)
dandischema.models,DandiBaseModel.unvalidated
pydantic.BaseModel.model_construct
Additional notes regarding changes entailed by Pydantic V2:
dandischema.models.CommonModel.repository
in APIs which expect astr
, usestr()
to convert the attribute to astr
.pydantic.networks
no longer inherit fromstr
.Optional
fields now indicate that the valuenull
is allowed. Because this change in behavior is incompatible with required tests for dandi-schema,dandischema.utils.TransitionalGenerateJsonSchema
has been defined to revert the behavior of the JSON schema generation forOptional
fields back to the behavior exhibited by Pydantic V1.null
value as for Pydantic V1, initiate the generation withModel.model_json_schema(schema_generator=TransitionalGenerateJsonSchema)
.GenerateJsonSchema
class, please checkout these examples.CommonModel
now has "description" instead of "descriptions" due to a typo correction I put in. (This is not a change entailed by Pydantic V2 but from a correction I put in the code.)AnyHttpUrl
. This is possibly due to the fact that all URL types in Pydantic V2 no longer inherit fromstr
and there is no max length restriction.Literal
type of a single value, the "type" key is no longer specified in the corresponding JSON schema. For example, the JSON schema for a field of the typeLiteral["Asset"]
no longer contains the "type" key with the value of "string". This change affects fields such asBareAsset.schemaKey
.Enum
subclass that hasstr
values for corresponding choices, the new JSON schema will include a"type"
key with the value of"string"
. This change affects pretty much all theEnum
subclasses defined in this project."description"
key in the JSON schema. For example, the JSON schema forProject
no longer has the"description"
key because its parent,Activity
, has a docstring,"""Information about the Project activity"""
.Note: This PR contains the "minimum" changes needed to support Pydantic V2. Other improvements to the project enabled by Pydantic V2 will come as subsequent PRs.
TODOs: