Add support for ConstrainedStr as dict keys #332

tiangolo · 2018-12-27T09:24:29Z

Change Summary

Add support for ConstrainedStr as dict keys.

Related issue number

Performance Changes

pydantic cares about performance, if there's any risk performance changed on this PR,
please run make benchmark-pydantic before and after the change:

before: ?
after: ?

Checklist

Unit tests for the changes exist
Tests pass on CI and coverage remains at 100%
Documentation reflects the changes
No performance deterioration (if applicable)
HISTORY.rst has been updated
- if this is the first change since a release, please add a new section
- include the issue number or this pull request number #<number>
- if you're not a regular contributer please include your github username @whatever

codecov · 2018-12-27T09:27:09Z

Codecov Report

Merging #332 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #332   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          13     13           
  Lines        1779   1783    +4     
  Branches      346    347    +1     
=====================================
+ Hits         1779   1783    +4

tiangolo · 2018-12-27T09:53:35Z

@samuelcolvin should I rebase to solve HISTORY conflicts?

samuelcolvin · 2018-12-27T09:59:46Z

Yes, or merge with master.

samuelcolvin

Otherwise LGTM.

samuelcolvin · 2018-12-27T10:11:48Z

pydantic/schema.py

-        else:
-            # The dict values are Any, no need to declare it
-            return {'type': 'object'}, definitions
+        if key_pattern:


no need for key_pattern just if regex here.

Agreed. Done.

samuelcolvin · 2018-12-27T10:12:55Z

pydantic/schema.py

+        if key_pattern:
+            # Dict keys have a regex pattern
+            # f_schema might be a schema or empty dict, add it either way
+            dict_schema.update({'patternProperties': {key_pattern: f_schema}})


since we're only setting on item use setitem dict_schema['patternProperties'] here and below.

Agreed. Done.

samuelcolvin · 2018-12-27T11:16:06Z

pydantic/schema.py

+            # Dict keys have a regex pattern
+            # f_schema might be a schema or empty dict, add it either way
+            dict_schema['patternProperties'] = {regex.pattern: f_schema}
+        elif f_schema:


Sorry, one more question.

Are these two cases really mutually exclusive? Eg. requiring elif rather than if.

maybe you might want both patternProperties and additionalProperties?

Yep. I thought about the same while implementing it.

The schema that would go in additionalProperties ends up as the value of the dict at patternProperties. That dict has as key the regex pattern, and as value the schema for the dict value.

Also from the spec:

Validation with "additionalProperties" applies only to the child values of instance names that do not match any names in "properties", and do not match any regular expression in "patternProperties".

I can think of a case that would allow for patternProperties and additionalProperties, but it is rather complex.

>>> from typing import Union, Dict, Any >>> from pydantic import BaseModel, constr >>> >>> Identifier = constr(regex=r'^([a-zA-Z_][a-zA-Z0-9_]*)$') >>> Slug = constr(regex=r'^([a-zA-Z_-][a-zA-Z0-9_-]*)$') >>> Number = constr(regex=r'^\d+$') >>> >>> class IdentifierModel(BaseModel): ... alias: str = ... ... >>> class SlugModel(BaseModel): ... alias: str = ... ... value: str = ... ... >>> class NumberModel(BaseModel): ... alias: str = ... ... value: int = ... ... >>> class Model(BaseModel): ... items: Union[Dict[Identifier, IdentifierModel], Dict[Slug, SlugModel], Dict[Number, NumberModel], Dict[str, Any]] = {} ... >>> print(Model.schema_json(indent=2)) { "title": "Model", "type": "object", "properties": { "items": { "title": "Items", "default": {}, "anyOf": [ { "type": "object", "patternProperties": { "^([a-zA-Z_][a-zA-Z0-9_]*)$": { "$ref": "#/definitions/IdentifierModel" } } }, { "type": "object", "patternProperties": { "^([a-zA-Z_-][a-zA-Z0-9_-]*)$": { "$ref": "#/definitions/SlugModel" } } }, { "type": "object", "patternProperties": { "^\\d+$": { "$ref": "#/definitions/NumberModel" } } }, { "type": "object" } ] } }, "definitions": { "IdentifierModel": { "title": "IdentifierModel", "type": "object", "properties": { "alias": { "title": "Alias", "type": "string" } }, "required": [ "alias" ] }, "SlugModel": { "title": "SlugModel", "type": "object", "properties": { "alias": { "title": "Alias", "type": "string" }, "value": { "title": "Value", "type": "string" } }, "required": [ "alias", "value" ] }, "NumberModel": { "title": "NumberModel", "type": "object", "properties": { "alias": { "title": "Alias", "type": "string" }, "value": { "title": "Value", "type": "integer" } }, "required": [ "alias", "value" ] } } } # expected { "title": "Model", "type": "object", "properties": { "items": { "title": "Items", "default": {}, "patternProperties": { "^([a-zA-Z_][a-zA-Z0-9_]*)$": { "$ref": "#/definitions/IdentifierModel" }, "^([a-zA-Z_-][a-zA-Z0-9_-]*)$": { "$ref": "#/definitions/SlugModel" }, "^\\d+$": { "$ref": "#/definitions/NumberModel" } }, "additionalProperties": {}, } }, "definitions": { "IdentifierModel": { "title": "IdentifierModel", "type": "object", "properties": { "alias": { "title": "Alias", "type": "string" } }, "required": [ "alias" ] }, "SlugModel": { "title": "SlugModel", "type": "object", "properties": { "alias": { "title": "Alias", "type": "string" }, "value": { "title": "Value", "type": "string" } }, "required": [ "alias", "value" ] }, "NumberModel": { "title": "NumberModel", "type": "object", "properties": { "alias": { "title": "Alias", "type": "string" }, "value": { "title": "Value", "type": "integer" } }, "required": [ "alias", "value" ] } } }

Good find, however I think this is too niche for now, if you think it's a problem @demosdemon please create a new issue to discuss.

Support for `patternProperties` was introduced in pydantic#332, but that logic unfortunately made `patternProperties` and `additionalProperties` mutually exclusive. JSON Schema supports their combination: https://json-schema.org/understanding-json-schema/reference/object.html#additional-properties This prevented well-typed code generation for dictionaries that use regex-constrained string keys.

…4641) Support for `patternProperties` was introduced in #332, but that logic unfortunately made `patternProperties` and `additionalProperties` mutually exclusive. JSON Schema supports their combination: https://json-schema.org/understanding-json-schema/reference/object.html#additional-properties This prevented well-typed code generation for dictionaries that use regex-constrained string keys.

* Integrate CodSpeed benchmarks with github actions * Apply suggestions from code review Co-authored-by: Samuel Colvin <samcolvin@gmail.com> * bump Co-authored-by: Arthur Pastel <arthur.pastel@gmail.com>

tiangolo mentioned this pull request Dec 27, 2018

Allow ConstrainedStr as K in Dict[K, V] types #329

Closed

tiangolo added 3 commits December 27, 2018 14:04

Add support for ConstrainedStr as dict keys, #fix pydantic#329

b02b64d

Update History with PR number

28266f3

Remove test for nonexisting case, raise coverage

a267c57

tiangolo force-pushed the dict-constr-key branch from c39c241 to a267c57 Compare December 27, 2018 10:05

samuelcolvin reviewed Dec 27, 2018

View reviewed changes

Simplify constr dict key implementation, as per code review

09ae7f4

samuelcolvin reviewed Dec 27, 2018

View reviewed changes

Merge branch 'master' into dict-constr-key

b009d48

samuelcolvin merged commit c6d1a69 into pydantic:master Dec 27, 2018

tiangolo deleted the dict-constr-key branch December 28, 2018 04:00

jparise mentioned this pull request Oct 22, 2022

Allow dicts to have both patternProperties and additionalProperties #4641

Merged

jparise mentioned this pull request Oct 22, 2022

Allow dicts to have both patternProperties and additionalProperties #4642

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add support for ConstrainedStr as dict keys #332

Add support for ConstrainedStr as dict keys #332

Uh oh!

tiangolo commented Dec 27, 2018

Uh oh!

codecov bot commented Dec 27, 2018 •

edited

Loading

Uh oh!

tiangolo commented Dec 27, 2018

Uh oh!

samuelcolvin commented Dec 27, 2018

Uh oh!

samuelcolvin left a comment

Uh oh!

samuelcolvin Dec 27, 2018

Uh oh!

tiangolo Dec 27, 2018

Uh oh!

samuelcolvin Dec 27, 2018

Uh oh!

tiangolo Dec 27, 2018

Uh oh!

samuelcolvin Dec 27, 2018 •

edited

Loading

Uh oh!

tiangolo Dec 27, 2018

Uh oh!

demosdemon Dec 27, 2018

Uh oh!

samuelcolvin Dec 27, 2018

Uh oh!

Uh oh!

Uh oh!

Add support for ConstrainedStr as dict keys #332

Add support for ConstrainedStr as dict keys #332

Uh oh!

Conversation

tiangolo commented Dec 27, 2018

Change Summary

Related issue number

Performance Changes

Checklist

Uh oh!

codecov bot commented Dec 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tiangolo commented Dec 27, 2018

Uh oh!

samuelcolvin commented Dec 27, 2018

Uh oh!

samuelcolvin left a comment

Choose a reason for hiding this comment

Uh oh!

samuelcolvin Dec 27, 2018

Choose a reason for hiding this comment

Uh oh!

tiangolo Dec 27, 2018

Choose a reason for hiding this comment

Uh oh!

samuelcolvin Dec 27, 2018

Choose a reason for hiding this comment

Uh oh!

tiangolo Dec 27, 2018

Choose a reason for hiding this comment

Uh oh!

samuelcolvin Dec 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tiangolo Dec 27, 2018

Choose a reason for hiding this comment

Uh oh!

demosdemon Dec 27, 2018

Choose a reason for hiding this comment

Uh oh!

samuelcolvin Dec 27, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Dec 27, 2018 •

edited

Loading

samuelcolvin Dec 27, 2018 •

edited

Loading