Skip to content

Commit

Permalink
Fix "Schema Key" becoming an entry in the meditor
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@mvandenburgh
mvandenburgh committed Feb 9, 2024
1 parent cd5cd04 commit b3db231
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion web/src/components/Meditor/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export const isJSONSchema = (schema: JSONSchemaUnionType): schema is JSONSchema7

export const isBasicSchema = (schema: JSONSchemaUnionType): schema is BasicSchema => (
isJSONSchema(schema)
&& isBasicType(schema.type)
&& (isBasicType(schema.type) || schema.type === undefined)
);

export const isObjectSchema = (schema: JSONSchemaUnionType): schema is ObjectSchema => (
Expand Down

5 comments on commit b3db231

@satra
Copy link
Member

@satra satra commented on b3db231 Feb 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @mvandenburgh . @candleindark let's check if there is something we can do in the model for schemaKey to have a behavior as before. if it cannot, let's consider removing it as a basic schema during generation.

@candleindark
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@satra If my guess is correct, I think you want to have the"type" key and its corresponding value back for the JSON-schema for the schemaKey field of each model. This is the change mentioned in 3v in the top post of dandi/dandi-schema#203. If that's is the case, I believe I can customize the JSON schema generation to match the JSON schema in pervious version.

Be aware though, this is a customization, and we are getting away from the standard behavior of JSON schema generation of Pydantic V2.

@candleindark
Copy link
Member

@candleindark candleindark commented on b3db231 Feb 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@satra Please checkout PR dandi/dandi-schema#227. I think it addresses the problem.

@candleindark
Copy link
Member

@candleindark candleindark commented on b3db231 Feb 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@satra Values allowed to be in a Literal expression must be a basic type in JSON, and values of different basic types in JSON are expressed differently. So Reverting the behavior of the schemaKey to have type field is not really needed if you can construe the type of the value from the expression of the value in JSON. In Python, json.dumps does this as illustrated in the following example. If there is a parallel behavior in TypeScript, then dandi/dandi-schema#227 is not needed.

import json


d_to_json = dict(a="1", b=2, c=[1, 2, 3], d=True, e=None, f=3.1415)

d_in_json = json.dumps(d_to_json, indent=2)

print(d_in_json)

d_from_json = json.loads(d_in_json)

assert d_to_json == d_from_json

@satra
Copy link
Member

@satra satra commented on b3db231 Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then let's keep the new behavior and update the meditor to address it as @mvandenburgh has done.

Please sign in to comment.