Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix parsing BigInt from str #1204

Merged
merged 2 commits into from
Mar 5, 2024
Merged

Fix parsing BigInt from str #1204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6a3c51e
stash changes
sydney-runkle Feb 22, 2024
e2c0184
fix big int parsing problems from str
sydney-runkle Feb 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/input/input_json.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,10 +370,7 @@ impl<'a> Input<'a> for String {
}

fn validate_int(&'a self, _strict: bool) -> ValResult<ValidationMatch<EitherInt<'a>>> {
match self.parse() {
Ok(i) => Ok(ValidationMatch::lax(EitherInt::I64(i))),
Err(_) => Err(ValError::new(ErrorTypeDefaults::IntParsing, self)),
}
str_as_int(self, self).map(ValidationMatch::lax)
}

fn validate_float(&'a self, _strict: bool) -> ValResult<ValidationMatch<EitherFloat<'a>>> {
Expand Down
7 changes: 2 additions & 5 deletions src/input/input_string.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::validators::decimal::create_decimal;
use super::datetime::{
bytes_as_date, bytes_as_datetime, bytes_as_time, bytes_as_timedelta, EitherDate, EitherDateTime, EitherTime,
};
use super::shared::{str_as_bool, str_as_float};
use super::shared::{str_as_bool, str_as_float, str_as_int};
use super::{
BorrowInput, EitherBytes, EitherFloat, EitherInt, EitherString, EitherTimedelta, GenericArguments, GenericIterable,
GenericIterator, GenericMapping, Input, ValidationMatch,
Expand Down Expand Up @@ -112,10 +112,7 @@ impl<'a> Input<'a> for StringMapping<'a> {

fn validate_int(&'a self, _strict: bool) -> ValResult<ValidationMatch<EitherInt<'a>>> {
match self {
Self::String(s) => match py_string_str(s)?.parse() {
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),
Copy link
Member

Choose a reason for hiding this comment

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

who is one ValidationMatch::strict and one ValidationMatch::lax?

Copy link
Member

Choose a reason for hiding this comment

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

maybe add a test for both cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure, though I just kept the logic consistent with what was there before...

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Self::Mapping(_) => Err(ValError::new(ErrorTypeDefaults::IntType, self)),
}
}
Expand Down
8 changes: 8 additions & 0 deletions tests/validators/test_int.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,3 +503,11 @@ def test_allow_inf_nan_false_json() -> None:
v.validate_json('Infinity')
with pytest.raises(ValidationError, match=r'Input should be a finite number \[type=finite_number'):
v.validate_json('-Infinity')


def test_json_big_int_key():
v = SchemaValidator({'type': 'dict', 'keys_schema': {'type': 'int'}, 'values_schema': {'type': 'str'}})
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'}
Copy link
Member

Choose a reason for hiding this comment

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

definitely worth adding a test for valdiate_string.

Copy link
Member Author

Choose a reason for hiding this comment

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

@samuelcolvin,

Did you have something different in mind than the line above?

assert v.validate_strings({str(big_integer): 'x'}) == {big_integer: 'x'}

Loading