-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
BUG: Fix json_normalize 'meta' validation #63136
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.
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
BUG: Fix json_normalize 'meta' validation #63136
Conversation
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
Updated the _validate_meta function to accept a string or a list of strings/lists of strings as input.
|
@rhshadrach could you please take a look at this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! It seems to me the meta argument is not expected to be large data (e.g. hundreds of entries) and so these instance checks are okay. Also it seems questionable to me whether this should go through deprecation. I'd like another eye here - cc @mroeschke
Removed tests for non-string meta in json_normalize.
Update _validate_meta function to accept None as a valid input.
mroeschke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM merge when ready @rhshadrach
rhshadrach
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
Thanks @nocoding03 |
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.AGENTS.md.Currently,
json_normalizeexhibits inconsistent behavior when using non-string keys in themetaparameter:meta=[12](withoutrecord_path) - incorrectly worksmeta=[12](withrecord_path) - correctly fails with TypeErrorThis inconsistency violates the principle of least surprise and contradicts the documented API.