Add support for propertyNames in JSON schema
#10478
Conversation
JSON schema draft 2020-12 defines a propertyNames key for dict/object schemas which can contain any valid JSON schema that defines to what any property name of the object instance needs to validate to. This can be mapped to a pydantic/python typing like dict[AnyEnum, str] where AnyEnum is an Enum type with defined values. propertyNames supports other validation schemas as well, e.g. a pattern. However, this is not covered with this commit. Fixes pydantic#4393
|
please review |
CodSpeed Performance ReportMerging #10478 will not alter performanceComparing Summary
|
|
Sorry for the additional commits, I haven't tested any other python version than 3.12, which I've installed locally 😬 |
Coverage report
This PR does not seem to contain any modification to coverable code. |
pydantic/json_schema.py
Outdated
| if ks is not None: | ||
| keys_type = ks.get('type', None) | ||
| if keys_type == 'enum': | ||
| keys_members = ks.get('members', []) | ||
| json_schema['propertyNames'] = {'enum': keys_members} |
There was a problem hiding this comment.
I think we can add support for any key schema out of the box
| if ks is not None: | |
| keys_type = ks.get('type', None) | |
| if keys_type == 'enum': | |
| keys_members = ks.get('members', []) | |
| json_schema['propertyNames'] = {'enum': keys_members} | |
| # Add `keys_schema.pop('title', None)` next to `values_schema.pop('title', None)` above and update comment | |
| if keys_schema.get('type') == 'string' and len(keys_schema) > 1: # len check means there are additional constraints to the key schema | |
| json_schema['propertyNames'] = keys_schema |
?
There was a problem hiding this comment.
Thanks for the review and the suggestion :) Just for better understanding from my side:
keys_schema could be a ref only, which means that I would need to resolve that before I can do any checks on that, right?
Would it then be best to add the ref to propertyNames schema, or the resolved one? I'll go with the resolved for now but am open for your inputs :)
There was a problem hiding this comment.
While doing it, this will also add a propertyNames schema to an existing test case. This should be expected behaviour, as python's Path in json schema of pydantic has the ' format' 'path' schema.
I just have to add this to the existing test case, and I'm not sure how such changes to existing behaviour is handled in pydantic tbh :)
There was a problem hiding this comment.
Hum right, although I'm not sure schemas of type 'string' can be made references? cc @sydney-runkle
There was a problem hiding this comment.
While doing it, this will also add a propertyNames schema to an existing test case. This should be expected behaviour, as python's Path in json schema of pydantic has the ' format' 'path' schema.
Agreed, this change looks correct to me. I second @Viicos' request for that additional test case with constrained dict keys.
Would it then be best to add the ref to propertyNames schema, or the resolved one? I'll go with the resolved for now but am open for your inputs :)
Re refs - I think the current behavior makes sense. I haven't seen strings as references in JSON schemas yet. Maybe @adriangb has?
There was a problem hiding this comment.
Would it then be best to add the ref to propertyNames schema, or the resolved one?
Looked closer at this - I think we can avoid the ref resolution. Can change if it comes up for a specific use case.
Thanks for the reviews :) I added the changes in the latest commit. |
propertyNames in JSON schema
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
|
Thanks for all your support and the reviews 🙏 |
|
Ah, I see why my change broke things. Need to think a bit more about this. There's a part of me that thinks this should be more unconditional, like: json_schema: JsonSchemaValue = {'type': 'object'}
keys_schema = self.generate_inner(schema['keys_schema']).copy() if 'keys_schema' in schema else {}
keys_pattern = keys_schema.pop('pattern', None)
values_schema = self.generate_inner(schema['values_schema']).copy() if 'values_schema' in schema else {}
values_schema.pop('title', None) # don't give a title to the additionalProperties
if values_schema or keys_pattern is not None: # don't add additionalProperties if it's empty
if keys_pattern is None:
json_schema['additionalProperties'] = values_schema
else:
json_schema['patternProperties'] = {keys_pattern: values_schema}
keys_schema.pop('title', None) # don't give a title to the propertyNames
if keys_schema:
json_schema['propertyNames'] = keys_schemaAnd we can just use refs like we would for other types. Need to chat with @Viicos before we move forward here. Thanks for your help @FlorianSW, we can take it from here :). |
|
@sydney-runkle, with your proposed change, class Model(BaseModel):
f: dict[Annotated[int, Field(gt=1)], str]
Model(f={2: "a"}).model_dump_json()
#> '{"f": {"2": "a"}}'While such keys are converted to strings when dumping to JSON, we shouldn't add a The len check feels a bit hacky, but I don't know if there's a better way to do so 🤔 |
|
Ah I see, I'm alright with just adding for I'm just hesitant to eagerly resolve the schema refs here - we don't do that elsewhere, and I'm afraid we'll run into errors with available defs at this point in the schema building process (I suppose we can fix and ignore with a catch, though that seems like an annoyingly silent failure). Thoughts on this @Viicos? |
|
So, I'm ok with this resolution ahead of time, as we do this for discriminator refs eagerly :). See pydantic/pydantic/json_schema.py Lines 1204 to 1206 in c9c9277 |
Change Summary
JSON schema draft 2020-12 defines a propertyNames key for dict/object schemas which can contain any valid JSON schema that defines to what any property name of the object instance needs to validate to.
This can be mapped to a pydantic/python typing like dict[AnyEnum, str] where AnyEnum is an Enum type with defined values.
propertyNames supports other validation schemas as well, e.g. a pattern. However, this is not covered with this PR.
Related issue number
Fixes #4393
Checklist
Selected Reviewer: @sydney-runkle