Skip to content

BUG: Fix nested_to_record with None values in nested levels #21164

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

Merged
merged 2 commits into from
Jun 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions doc/source/whatsnew/v0.23.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ I/O

- Bug in IO methods specifying ``compression='zip'`` which produced uncompressed zip archives (:issue:`17778`, :issue:`21144`)
- Bug in :meth:`DataFrame.to_stata` which prevented exporting DataFrames to buffers and most file-like objects (:issue:`21041`)
- Bug when :meth:`pandas.io.json.json_normalize` was called with ``None`` values in nested levels in JSON (:issue:`21158`)
- Bug in :meth:`DataFrame.to_csv` and :meth:`Series.to_csv` causes encoding error when compression and encoding are specified (:issue:`21241`, :issue:`21118`)
- Bug in :meth:`read_stata` and :class:`StataReader` which did not correctly decode utf-8 strings on Python 3 from Stata 14 files (dta version 118) (:issue:`21244`)
-
Expand Down
2 changes: 1 addition & 1 deletion pandas/io/json/normalize.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def nested_to_record(ds, prefix="", sep=".", level=0):
if level != 0: # so we skip copying for top level, common case
v = new_d.pop(k)
new_d[newkey] = v
if v is None: # pop the key if the value is None
elif v is None: # pop the key if the value is None
new_d.pop(k)
continue
else:
Expand Down
56 changes: 56 additions & 0 deletions pandas/tests/io/json/test_normalize.py
Original file line number Diff line number Diff line change
Expand Up @@ -375,3 +375,59 @@ def test_nonetype_dropping(self):
'info.last_updated': '26/05/2012'}]

assert result == expected

def test_nonetype_top_level_bottom_level(self):
# GH21158: If inner level json has a key with a null value
# make sure it doesnt do a new_d.pop twice and except
data = {
"id": None,
"location": {
"country": {
"state": {
"id": None,
"town.info": {
"id": None,
"region": None,
"x": 49.151580810546875,
"y": -33.148521423339844,
"z": 27.572303771972656}}}
}
}
result = nested_to_record(data)
expected = {
'location.country.state.id': None,
'location.country.state.town.info.id': None,
'location.country.state.town.info.region': None,
'location.country.state.town.info.x': 49.151580810546875,
'location.country.state.town.info.y': -33.148521423339844,
'location.country.state.town.info.z': 27.572303771972656}
assert result == expected

def test_nonetype_multiple_levels(self):
# GH21158: If inner level json has a key with a null value
# make sure it doesnt do a new_d.pop twice and except
data = {
"id": None,
Copy link
Contributor

Choose a reason for hiding this comment

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

does this have the same result if id is NOT repeated (with None), just missing at various levels? e.g. try with a single id with None at the top and bottom levels (in another case, leave this one as well)

"location": {
"id": None,
"country": {
"id": None,
"state": {
"id": None,
"town.info": {
"region": None,
"x": 49.151580810546875,
"y": -33.148521423339844,
"z": 27.572303771972656}}}
}
}
result = nested_to_record(data)
expected = {
'location.id': None,
Copy link
Member

Choose a reason for hiding this comment

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

I was reading through the commentary of the issue and noticed there was some confusion on the topic, but I don't understand why we would want to drop the 'id': None record here - is that solely driven by the elif statement? If so, perhaps we don't need that at all?

Choose a reason for hiding this comment

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

I know I'm late replying to this, but I agree. I don't understand the intention of dropping the top level 'id': None. I would much rather know explicitly that the value for a field is None than to be guessing or writing extra checks for whether the field exists.

'location.country.id': None,
'location.country.state.id': None,
'location.country.state.town.info.region': None,
'location.country.state.town.info.x': 49.151580810546875,
'location.country.state.town.info.y': -33.148521423339844,
'location.country.state.town.info.z': 27.572303771972656}
assert result == expected