Fix #21356: JSON nested_to_record Silently Drops Top-Level None Values #21363

daminisatya · 2018-06-07T18:25:30Z

closes JSON nested_to_record Silently Drops Top-Level None Values #21356
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

daminisatya · 2018-06-07T18:26:41Z

@WillAyd Kindly review, this is my first PR to pandas!

WillAyd

Looks really good for a first PR. Just some minor comments on the comments. Otherwise can you add a whatsnew note for 0.24?

WillAyd · 2018-06-07T18:40:54Z

pandas/tests/io/json/test_normalize.py

@@ -242,11 +242,13 @@ def test_missing_field(self, author_missing_data):
        # unnest records where only the first record has a None value


Can delete this comment (tbh I don't really understand what it means even with the current behavior)

WillAyd · 2018-06-07T18:41:41Z

pandas/tests/io/json/test_normalize.py

@@ -351,7 +353,7 @@ def test_json_normalize_errors(self):
                      errors='raise'
                      )

-    def test_nonetype_dropping(self):
+    def test_nonetype(self):
        # GH20030: Checks that None values are dropped in nested_to_record


Can certainly delete this comment, though replace with a reference to the GitHub issue you are working on

…None Values

daminisatya · 2018-06-07T19:15:04Z

pandas/tests/io/json/test_normalize.py

@@ -238,15 +238,16 @@ def test_non_ascii_key(self):
        tm.assert_frame_equal(result, expected)

    def test_missing_field(self, author_missing_data):
-        # GH20030: Checks for robustness of json_normalize - should
-        # unnest records where only the first record has a None value
+        # GH20030: Checks for robustness of json_normalize


@WillAyd Can you verify this comment once again.

Just the GH issue number is sufficient (like you have below)

daminisatya · 2018-06-07T19:15:20Z

pandas/tests/io/json/test_normalize.py

-        # GH20030: Checks that None values are dropped in nested_to_record
-        # to prevent additional columns of nans when passed to DataFrame
+    def test_nonetype(self):
+        # GH21356


Here, i just added the issue number.

daminisatya · 2018-06-07T19:15:48Z

doc/source/whatsnew/v0.24.0.txt

@@ -80,6 +80,8 @@ Documentation Changes
 Bug Fixes
 ~~~~~~~~~

+- The top level None value is not dropped but rather preserved along with lower levels for consistency (:issue:`21356`)


Added the bugfix in the whats new note.

Change of plans here - can you add to the "Fixed Regressions" section in v0.23.1 introduced via #21368?

this also needs to mention json_normalize, add double backticks on None.

finally, as a user this is not really clear on what has changed, can you re-word a bit.

daminisatya

Made the requested changes.

WillAyd

Minor edits otherwise lgtm

WillAyd · 2018-06-07T21:20:49Z

doc/source/whatsnew/v0.24.0.txt

@@ -80,6 +80,8 @@ Documentation Changes
 Bug Fixes
 ~~~~~~~~~

+- The top level None value is not dropped but rather preserved along with lower levels for consistency (:issue:`21356`)


Change of plans here - can you add to the "Fixed Regressions" section in v0.23.1 introduced via #21368?

WillAyd · 2018-06-07T21:21:03Z

pandas/tests/io/json/test_normalize.py

@@ -238,15 +238,16 @@ def test_non_ascii_key(self):
        tm.assert_frame_equal(result, expected)

    def test_missing_field(self, author_missing_data):
-        # GH20030: Checks for robustness of json_normalize - should
-        # unnest records where only the first record has a None value
+        # GH20030: Checks for robustness of json_normalize


Just the GH issue number is sufficient (like you have below)

codecov · 2018-06-07T23:39:40Z

Codecov Report

❗ No coverage uploaded for pull request base (master@636dd01). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #21363   +/-   ##
=========================================
  Coverage          ?   91.85%           
=========================================
  Files             ?      153           
  Lines             ?    49562           
  Branches          ?        0           
=========================================
  Hits              ?    45525           
  Misses            ?     4037           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.25% <ø> (?)`
#single	`41.86% <ø> (?)`

Impacted Files	Coverage Δ
pandas/io/json/normalize.py	`96.87% <ø> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 636dd01...b7bda86. Read the comment docs.

jreback · 2018-06-08T11:06:48Z

doc/source/whatsnew/v0.24.0.txt

@@ -80,6 +80,8 @@ Documentation Changes
 Bug Fixes
 ~~~~~~~~~

+- The top level None value is not dropped but rather preserved along with lower levels for consistency (:issue:`21356`)


this also needs to mention json_normalize, add double backticks on None.

finally, as a user this is not really clear on what has changed, can you re-word a bit.

jreback · 2018-06-08T11:07:13Z

pandas/tests/io/json/test_normalize.py

-    def test_nonetype_dropping(self):
-        # GH20030: Checks that None values are dropped in nested_to_record
-        # to prevent additional columns of nans when passed to DataFrame
+    def test_nonetype(self):


can you make this test name more informative

…s-dev#21367)

This reverts commit 7c7bd56.

)

…das-dev#21183)

jreback · 2018-06-08T16:30:08Z

@WillAyd if you have a few, can you rebase this

WillAyd · 2018-06-08T16:38:58Z

@jreback think its good to go. First time doing that so let me know if anything looks off

jorisvandenbossche · 2018-06-08T16:50:32Z

@daminisatya Thanks a lot!

…None Values (pandas-dev#21363) (cherry picked from commit ff26632)

#21363) (cherry picked from commit ff26632)

…None Values (pandas-dev#21363)

WillAyd requested changes Jun 7, 2018

View reviewed changes

daminisatya force-pushed the master branch from 99b745e to b2ded06 Compare June 7, 2018 19:06

Fix pandas-dev#21356: JSON nested_to_record Silently Drops Top-Level …

2dbf9f2

…None Values

daminisatya force-pushed the master branch from b2ded06 to 2dbf9f2 Compare June 7, 2018 19:09

Added bugfix pandas-dev#21356 to whatsnew doc under 0.24v

2ac19b5

daminisatya commented Jun 7, 2018

View reviewed changes

jorisvandenbossche added the IO JSON read_json, to_json, json_normalize label Jun 7, 2018

jorisvandenbossche added this to the 0.23.1 milestone Jun 7, 2018

WillAyd requested changes Jun 7, 2018

View reviewed changes

jreback requested changes Jun 8, 2018

View reviewed changes

daminisatya and others added 9 commits June 8, 2018 09:03

changes requested

0de0cd4

DOC: move whatsnew file for pandas-dev#21116 (index droplevel) (panda…

832cc56

…s-dev#21367)

DOC: clean-up 0.23.1 whatsnew (pandas-dev#21368)

a1858c2

BUG: Fixed concat warning message (pandas-dev#21362)

e64acdf

Revert "enable multivalues insert (pandas-dev#19664)" (pandas-dev#21355)

2fa7818

This reverts commit 7c7bd56.

BUG: dropna incorrect with categoricals in pivot_table (pandas-dev#21252

f3f7eb9

)

Sharey keyword for boxplot (pandas-dev#20968)

7471f3e

BUG: Series.combine() fails with ExtensionArray inside of Series (pan…

1027a1c

…das-dev#21183)

requested changes added

7bc2124

WillAyd added 2 commits June 8, 2018 09:37

Merge branch 'master' into master

e7f787e

Update v0.23.1.txt

b7bda86

jorisvandenbossche merged commit ff26632 into pandas-dev:master Jun 8, 2018

TomAugspurger added the Needs Backport label Jun 12, 2018

TomAugspurger pushed a commit to TomAugspurger/pandas that referenced this pull request Jun 12, 2018

Fix pandas-dev#21356: JSON nested_to_record Silently Drops Top-Level …

17dad0d

…None Values (pandas-dev#21363) (cherry picked from commit ff26632)

TomAugspurger pushed a commit that referenced this pull request Jun 12, 2018

Fix #21356: JSON nested_to_record Silently Drops Top-Level None Values (

88260ea

#21363) (cherry picked from commit ff26632)

TomAugspurger removed the Needs Backport label Jun 12, 2018

david-liu-brattle-1 pushed a commit to david-liu-brattle-1/pandas that referenced this pull request Jun 18, 2018

Fix pandas-dev#21356: JSON nested_to_record Silently Drops Top-Level …

de162fc

…None Values (pandas-dev#21363)

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

Fix pandas-dev#21356: JSON nested_to_record Silently Drops Top-Level …

92c1c74

…None Values (pandas-dev#21363)

		@@ -242,11 +242,13 @@ def test_missing_field(self, author_missing_data):
		# unnest records where only the first record has a None value

Uh oh!

Fix #21356: JSON nested_to_record Silently Drops Top-Level None Values #21363

Fix #21356: JSON nested_to_record Silently Drops Top-Level None Values #21363

Uh oh!

Conversation

daminisatya commented Jun 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daminisatya commented Jun 7, 2018

Uh oh!

WillAyd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daminisatya left a comment

Choose a reason for hiding this comment

Uh oh!

WillAyd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jun 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback commented Jun 8, 2018

Uh oh!

WillAyd commented Jun 8, 2018

Uh oh!

jorisvandenbossche commented Jun 8, 2018

Uh oh!

Uh oh!

daminisatya commented Jun 7, 2018 •

edited

Loading

codecov bot commented Jun 7, 2018 •

edited

Loading