Add support for parsing boolean values in from_json
#9620
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…em being dropped if incomplete or invalid
andygrove
changed the title
from_jsonfrom_json
|
build |
revans2
reviewed
Nov 6, 2023
| @pytest.mark.parametrize('pattern', [ | ||
| r'{ "bool": [truefalsTRUEFALS]{1,5} }', | ||
| r'{ "bool": "[truefalsTRUEFALS]{1,5}" }', | ||
| pytest.param(r'{ "bool": [0-9]{0,2}(\.[0-9]{1,2})? }', marks=pytest.mark.xfail(reason='TBD')), |
There was a problem hiding this comment.
We need an actual issue for this, not just TBD
| .with_special_pattern('null', weight=50) | ||
| @pytest.mark.parametrize('pattern', [ | ||
| r'{ "bool": [truefalsTRUEFALS]{1,5} }', | ||
| r'{ "bool": "[truefalsTRUEFALS]{1,5}" }', |
There was a problem hiding this comment.
So what is the probability that this actually shows up as "true" or "false"? I think it has to be worse than 1 out of 524288 rows 2/(16^5) because that would be the probability for {5} at the end, but it is {1,5}. Those values should be returned as null because a quoted string should not be interpreted as a boolean, but I don't think this code is doing that. I could be wrong though, but the probabilities don't give me a lot of comfort that we have covered that test case.
There was a problem hiding this comment.
I have updated the tests and added a link to the issue
revans2
approved these changes
Nov 7, 2023
|
build |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #9597
Changes in this PR:
from_json