Fix parsing BigInt from str #1204
Conversation
CodSpeed Performance ReportMerging #1204 will not alter performanceComparing Summary
|
|
Please review |
| Ok(i) => Ok(ValidationMatch::strict(EitherInt::I64(i))), | ||
| Err(_) => Err(ValError::new(ErrorTypeDefaults::IntParsing, self)), | ||
| }, | ||
| Self::String(s) => str_as_int(self, py_string_str(s)?).map(ValidationMatch::strict), |
There was a problem hiding this comment.
who is one ValidationMatch::strict and one ValidationMatch::lax?
There was a problem hiding this comment.
maybe add a test for both cases.
There was a problem hiding this comment.
Not sure, though I just kept the logic consistent with what was there before...
There was a problem hiding this comment.
This will control union behaviour, to add test cases would need to put these in unions with str as the equivalent validator on the RHS, and confirm that in both cases the str leg wins?
e.g. dict[int, str] | dict[str, str] - I guess the dict[str, str] is the better choice.
There was a problem hiding this comment.
As discussed with @sydney-runkle offline, I think the divergence is a historical bug that's probably on me. Let's not block this PR on it.
| big_integer = 1433352099889938534014333520998899385340 | ||
| assert v.validate_python({big_integer: 'x'}) == {big_integer: 'x'} | ||
| assert v.validate_json('{"' + str(big_integer) + '": "x"}') == {big_integer: 'x'} | ||
| assert v.validate_strings({str(big_integer): 'x'}) == {big_integer: 'x'} |
There was a problem hiding this comment.
definitely worth adding a test for valdiate_string.
There was a problem hiding this comment.
Did you have something different in mind than the line above?
assert v.validate_strings({str(big_integer): 'x'}) == {big_integer: 'x'}
FIx pydantic/pydantic#8835
Selected Reviewer: @davidhewitt