[1.10.X] Fix field regex with StrictStr type annotation #4538
Conversation
There was a problem hiding this comment.
otherwise happy to accept this.
pydantic/types.py
Outdated
| regex = cls._regex() | ||
| if regex: | ||
| if not regex.match(value): |
There was a problem hiding this comment.
I'm afraid this might be slow since the compiled regex might not be cached.
Better to do the following which I think should work in both cases
| regex = cls._regex() | |
| if regex: | |
| if not regex.match(value): | |
| if cls.regex: | |
| if not re.match(cls.regex, value): |
pydantic/types.py
Outdated
|
|
||
| @classmethod | ||
| def __modify_schema__(cls, field_schema: Dict[str, Any]) -> None: | ||
| regex = cls._regex() |
There was a problem hiding this comment.
you'll therefore still need an isinstance check here.
|
@samuelcolvin Thanks for your review feedback! I think I've made the changes accordingly which should improve regex caching. Happy to receive your feedback again. 🙂 |
There was a problem hiding this comment.
LGTM.
@hramezani please review and check you're happy with this.
|
thanks so much for this. |
|
Sure thing, thanks for the review parallel to all the v2 dev work! Can't wait to see v2 and all its improvements! 😉 |
There's an inconsistency between
constr(...)andConstrainedStr, which is the base class ofStrictStr, with regard to the expected type of theregexargument/attribute which causes a field of typeStrictStrwith aregexconstraint specified via theField(...)class to raise an error:I've fixed this inconsistency in a backwards compatible way by extending the type of
regexinConstrainedStrto also support astrvalue in addition toPattern[str].I hope this fix will be considered despite the ongoing efforts towards Pydantic v2. 🙂