BUG: Fix nested_to_record with None values in nested levels
State: closed

- [ ] closes #21158
- [ ] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Continue after pop so you dont pop with the same key twice in a row



Labels:
name: IO JSON
description: read_json, to_json, json_normalize

Comments:
>>  git diff upstream/master -u -- "*.py" | flake8 --diff
$ git diff upstream/master -u -- "*.py" | flake8 --diff
$ 


Hello @ssikdar1! Thanks for updating the PR.





Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on June 07, 2018 at 12:16 Hours UTC
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=h1) Report
> Merging [#21164](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/172ab7aee433fed7780bf96f0c723af284755324?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/21164/graphs/tree.svg?height=150&width=650&token=eZ4WkYLtcO&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #21164      +/-   ##
==========================================
+ Coverage   91.84%   91.84%   +<.01%     
==========================================
  Files         153      153              
  Lines       49505    49506       +1     
==========================================
+ Hits        45466    45467       +1     
  Misses       4039     4039
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.23% <100%> (ø)` | :arrow_up: |
| #single | `41.88% <0%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/json/normalize.py](https://codecov.io/gh/pandas-dev/pandas/pull/21164/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2pzb24vbm9ybWFsaXplLnB5) | `96.96% <100%> (+0.03%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=footer). Last update [172ab7a...34c745e](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Can you check my latest commits? I changed it to not use a continue.

Also changed the whatsnew as well.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=h1) Report
> Merging [#21164](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/172ab7aee433fed7780bf96f0c723af284755324?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/21164/graphs/tree.svg?height=150&src=pr&token=eZ4WkYLtcO&width=650)](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master   #21164   +/-   ##
=======================================
  Coverage   91.84%   91.84%           
=======================================
  Files         153      153           
  Lines       49505    49505           
=======================================
  Hits        45466    45466           
  Misses       4039     4039
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.23% <100%> (ø)` | :arrow_up: |
| #single | `41.88% <0%> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/json/normalize.py](https://codecov.io/gh/pandas-dev/pandas/pull/21164/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2pzb24vbm9ybWFsaXplLnB5) | `96.93% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=footer). Last update [172ab7a...34c745e](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=h1) Report
> Merging [#21164](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/cea0a81b3d1ade61a5c662458dd8edc135dc94f6?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/21164/graphs/tree.svg?token=eZ4WkYLtcO&src=pr&width=650&height=150)](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master   #21164   +/-   ##
=======================================
  Coverage   91.85%   91.85%           
=======================================
  Files         153      153           
  Lines       49555    49555           
=======================================
  Hits        45518    45518           
  Misses       4037     4037
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.25% <100%> (ø)` | :arrow_up: |
| #single | `41.87% <0%> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/json/normalize.py](https://codecov.io/gh/pandas-dev/pandas/pull/21164/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2pzb24vbm9ybWFsaXplLnB5) | `96.93% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=footer). Last update [cea0a81...92a6263](https://codecov.io/gh/pandas-dev/pandas/pull/21164?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

kk, changed to elif
Moved into the I/O section in whatsnew. Changed the message hopefully its clearer.
@WillAyd 

>> Can you check if we have a test case to handle None values that show up in the initial level?

These test case below have a json list with a key as None in the initial level of a json.

https://github.com/pandas-dev/pandas/blob/master/pandas/tests/io/json/test_normalize.py#L354

  data = [
            {'info': None,
             'author_name':
             {'first': 'Smith', 'last_name': 'Appleseed'}
             },
            {'info':
                {'created_at': '11/08/1993', 'last_updated': '26/05/2012'},
             'author_name':
                {'first': 'Jane', 'last_name': 'Doe'}
             }
        ]

https://github.com/pandas-dev/pandas/blob/master/pandas/tests/io/json/test_normalize.py#L58

[
        {'info': None},
        {'info':
            {'created ...


Can you at least add a test with None in one of the middle layers? 

- I changed the test to be a test with None at multiple levels.
```
+        data = {
 +            "id": None,
 +            "location": {
 +                "id": None,
 +                "country": {
 +                    "id": None,
 +                    "state": {
 +                        "id": None,
 +                        "town.info": {
 +                            "region": None,
 +                            "x": 49.151580810546875,
 +                            "y": -33.148521423339844,
 +                            "z": 27.572303771972656}}}
 +            }
 +        }
```
Whatsnew:
Capitalized the b in Bug and put None in ticks

@WillAyd 

>>> 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?
>>>

So I took an example from here https://github.com/pandas-dev/pandas/issues/20030

```
import pandas as pd
data_partial_fail = \
        [{'info': None, 
         'author_name': 
         {'first': 'Smith', 'last_name': 'Appleseed'}
        }, 
        {'info': 
         {'created_at': '11/08/1993', 'last_updated': '26/05/2012'},
        'author_name': 
         {'first': 'Jane', 'last_name': 'Doe'}
        }]
p = pd.io.json.json_normalize(data_partial_fail)
print(p.columns)
```

With the my current pull request as is it is, it prints:
```
Index(['author_name.first', 'author_name.last_name', 'info.created_at',
       'info.last_updated'],
      dtype='object')
```
Now commenting out the elif, it prints:
```
(test) shan@shan-ThinkPad-T530:~/test$ python foo.py 
Index(['author_name.first', 'author_name.last_name', 'info', 'info.created_at',
       'info.last_updated'],
      dtype='object')
```

The issue here being the added column info.
The bug issue fix to 20030  added the original change in an attempt to get rid of the of the extra info column:
https://github.com/pandas-dev/pandas/pull/20399/files#diff-9c654764f5f21c8e9d58d9ebf14de86dR83

The change just didnt consider that a pop would occur twice in a row if the level > 0 and was also value was also None.

If we get rid of the elif we would undo issue 20030 from that perspective?

pls rebase as well
- Whatsnew changed to have `pandas.io.json.json_normalize`  and ``None`` 
- another testcase added to test the id: None at various levels
- rebased w/ master
looks ok to me. @WillAyd pls merge when you are satisifed.
@jreback somewhat conflicted on this. This PR certainly fixes the issue at hand, but what is the point of us dropping `None` values at the top level of the JSON yet maintaining it at deeper levels? Seems like it would be more consistent from an end user perspective to maintain at all levels and leave it to them to explicitly drop if they wanted.

Could probably go forward with this for 0.23.1 as the bug fix but then revisit top level `None`-handling for 0.24.0?
@WillAyd hmm, that's a good point. shouldn't ``id`` be in the expected? why are we dropping it at all? yeah that sees wrong
As @ssikdar1 points out it does stem from #20030, though in reading through that and the subsequent PR I think it is confusing what was actually being addressed. The PR would imply that automatically dropping the top-level `None` value is a feature, though I would argue that's not desirable and is in fact a bug.


@ssikdar1 can you rebase

@WillAyd ok with this?
Yep we can go with this for now. I'll open a separate issue for the top-level None parsing
Thanks @ssikdar1 !
No problem!