Repository: pandas-dev/pandas
Pull request: BUG: Fix json_normalize throwing TypeError when record_path has a sequence of dicts #22706
 
- [x] closes #22706
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Problem statement:
json_normalize raises TypeError exception
#### Code Sample, a copy-pastable example if possible

```python
from pandas.io.json import json_normalize

d = {
    'name': 'alan smith',    
    'info': {
        'phones': [{
            'area': 111,
            'number': 2222
        }, {
            'area': 333,
            'number': 4444
        }]
    }
}
json_normalize(d, record_path=["info", "phones"])
```
#### Problem description

The above code throws `TypeError` exception:
```
Traceback (most recent call last):
  File ".\test.py", line 15, in <module>
    json_normalize(d, record_path = ["info", "phones"])
  File "C:\Python36\lib\site-packages\pandas\io\json\normalize.py", line 262, in json_normalize
    _recursive_extract(data, record_path, {}, level=0)
  File "C:\Python36\lib\site-packages\pandas\io\json\normalize.py", line 235, in _recursive_extract
    seen_meta, level=level + 1)
  File "C:\Python36\lib\site-packages\pandas\io\json\normalize.py", line 238, in _recursive_extract
    recs = _pull_field(obj, path[0])
  File "C:\Python36\lib\site-packages\pandas\io\json\normalize.py", line 185, in _pull_field
    result = result[spec]
TypeError: string indices must be integers
```

#### Expected Output
| |area|number|
|-|-|-|
|0|111|2222|
|1|333|4444|
#### Output of ``pd.show_versions()``

<details>
INSTALLED VERSIONS
------------------
commit: None
python: 3.6.4.final.0
python-bits: 64
OS: Windows
OS-release: 10
machine: AMD64
processor: Intel64 Family 6 Model 62 Stepping 4, GenuineIntel
byteorder: little
LC_ALL: None
LANG: None
LOCALE: None.None

pandas: 0.23.4
pytest: 3.6.2
pip: 18.0
setuptools: 40.2.0
Cython: None
numpy: 1.14.5
scipy: None
pyarrow: None
xarray: None
IPython: 6.3.1
sphinx: 1.5.5
patsy: None
dateutil: 2.7.3
pytz: 2018.5
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: None
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: None
lxml: None
bs4: 4.6.0
html5lib: 1.0.1
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: 2.10
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None
</details>

Hints from issue discussion:
Thanks for the report - investigation and PRs are always welcome!
If `record_path` points to a nested dict of dicts, after one `_recursive_extract`, `data` is the inner dict (`{'phones': ...}` in the example)

When `data` is a dict, the for loop [here](https://github.com/pandas-dev/pandas/blob/1c500fb7b3fa08c163e13375d01b9607fcdac0d6/pandas/io/json/normalize.py#L237
) only iterates over the keys.

Do we assume that `data` is always a list? If that is the case, there are two options:
1. Turn data into a list if it is a dict (similar to line 194).
2. Hoist the for loop into a method. If data is not a list call this method instead of iterating over the elements.

I prefer (2). Let me know what you think. I can create a PR. 


@WillAyd : What do you think of the proposed fix? I'll create a PR if you think it's the right thing to do.
> Do we assume that data is always a list? If that is the case, there are two options:

The docstring claims that either a dict or list of dicts is allowed. The only example with a dict doesn't really do any normalization though:

```
>>> data = {'A': [1, 2]}
>>> json_normalize(data, 'A', record_prefix='Prefix.')
    Prefix.0
0          1
1          2
```

I'm inclined to do whatever is easiest to maintain in the long-run, though it's not clear what that is in this case.
I don't think we should assume that it is always a list. In my mind the behavior for `record_path` should mirror whatever happens at the top level but just resolving that at the specified `record_path`. These calls have an equivalent return:

```python
In [6]: json_normalize({'foo': 1, 'bar': 2, 'baz': 3})
Out[6]: 
   bar  baz  foo
0    2    3    1

In [7]: json_normalize([{'foo': 1, 'bar': 2, 'baz': 3}])
Out[7]: 
   bar  baz  foo
0    2    3    1
```

So I would assume the following to also be equivalent (though currently failing)

```python
>>> json_normalize({'info': {'phones': {'foo': 1, 'bar': 2, 'baz': 3}}}, record_path=['info', 'phones'])
>>> json_normalize({'info': {'phones': [{'foo': 1, 'bar': 2, 'baz': 3}]}}, record_path=['info', 'phones'])
To be clear, I asked about `data` in [`_recursive_extract`](https://github.com/pandas-dev/pandas/blob/1c500fb7b3fa08c163e13375d01b9607fcdac0d6/pandas/io/json/normalize.py#L227) (not the parameter `data` in `json_normalize`).

I agree with @WillAyd that the list assumption inside `_recursive_extract` is wrong. Inside this function `data` can be anything (list, dict, value). That's why my proposed fix above has a check to deal with non-list type. The proposed fix is as follows:
```python
def _extract(data, path, seen_meta, level):
    for obj in data: # the body of else clause at L237
        ...

def _recursive_extract(data, path, seen_meta, level=0):
    if len(path) > 1: 
        # unchanged
    else:
        if isinstance(data, list):
            for obj in data: # similar to the current version
                _extract(obj, path, seen_meta, level) 
        else:
            _extract(data, path, seen_meta, level) # this is new to deal with non-list data
```

Note that the current version is
```python
def _recursive_extract(data, path, seen_meta, level=0):
    if len(path) > 1: 
        # unchanged
    else:
        for obj in data: 
            _extract(obj, path, seen_meta, level) 
```
which raises exception when `data` is not a list.
@vuminhle feel free to submit a PR for code review
Btw I think #21605 has the same root cause.
PR #22804 should also fix this bug.

Commit message: Address feedback

Patch to review:
diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt
index 16f0b9ee99909..7103692a37f5f 100644
--- a/doc/source/whatsnew/v0.24.0.txt
+++ b/doc/source/whatsnew/v0.24.0.txt
@@ -932,6 +932,7 @@ I/O
 - :func:`read_sas()` will correctly parse sas7bdat files with data page types having also bit 7 set (so page type is 128 + 256 = 384) (:issue:`16615`)
 - Bug in :meth:`detect_client_encoding` where potential ``IOError`` goes unhandled when importing in a mod_wsgi process due to restricted access to stdout. (:issue:`21552`)
 - Bug in :func:`to_string()` that broke column alignment when ``index=False`` and width of first column's values is greater than the width of first column's header (:issue:`16839`, :issue:`13032`)
+- Bug in :func:`json_normalize` that caused it to raise ``TypeError`` when two consecutive elements of ``record_path`` are dicts (:issue:`22706`)
 
 Plotting
 ^^^^^^^^
diff --git a/pandas/io/json/normalize.py b/pandas/io/json/normalize.py
index 5c7b964cf69d1..ac09273629aa0 100644
--- a/pandas/io/json/normalize.py
+++ b/pandas/io/json/normalize.py
@@ -224,6 +224,44 @@ def _pull_field(js, spec):
         sep = str(sep)
     meta_keys = [sep.join(val) for val in meta]
 
+    def _extract(obj, field, seen_meta, level):
+        """
+        Extract field from obj and update `records`, `lengths`, `meta_vals`.
+
+        Parameters
+        ----------
+        obj : dict
+            Unserialized dict
+        field : str
+            The field to extract from obj
+        seen_meta : dict
+            The dict of meta values that have been visited
+        level: int
+            The current level
+        """
+        recs = _pull_field(obj, field)
+
+        # For repeating the metadata later
+        lengths.append(len(recs))
+
+        for val, key in zip(meta, meta_keys):
+            if level >= len(val):
+                meta_val = seen_meta[key]
+            else:
+                try:
+                    meta_val = _pull_field(obj, val[level:])
+                except KeyError as e:
+                    if errors == 'ignore':
+                        meta_val = np.nan
+                    else:
+                        raise KeyError("Try running with "
+                                       "errors='ignore' as key "
+                                       "{err} is not always present"
+                                       .format(err=e))
+            meta_vals[key].append(meta_val)
+
+        records.extend(recs)
+
     def _recursive_extract(data, path, seen_meta, level=0):
         if len(path) > 1:
             for obj in data:
@@ -233,30 +271,11 @@ def _recursive_extract(data, path, seen_meta, level=0):
 
                 _recursive_extract(obj[path[0]], path[1:],
                                    seen_meta, level=level + 1)
-        else:
+        elif isinstance(data, list):
             for obj in data:
-                recs = _pull_field(obj, path[0])
-
-                # For repeating the metadata later
-                lengths.append(len(recs))
-
-                for val, key in zip(meta, meta_keys):
-                    if level + 1 > len(val):
-                        meta_val = seen_meta[key]
-                    else:
-                        try:
-                            meta_val = _pull_field(obj, val[level:])
-                        except KeyError as e:
-                            if errors == 'ignore':
-                                meta_val = np.nan
-                            else:
-                                raise KeyError("Try running with "
-                                               "errors='ignore' as key "
-                                               "{err} is not always present"
-                                               .format(err=e))
-                    meta_vals[key].append(meta_val)
-
-                records.extend(recs)
+                _extract(obj, path[0], seen_meta, level)
+        else:
+            _extract(data, path[0], seen_meta, level)
 
     _recursive_extract(data, record_path, {}, level=0)
 
diff --git a/pandas/tests/io/json/test_normalize.py b/pandas/tests/io/json/test_normalize.py
index 200a853c48900..3881b315bbed9 100644
--- a/pandas/tests/io/json/test_normalize.py
+++ b/pandas/tests/io/json/test_normalize.py
@@ -129,6 +129,21 @@ def test_value_array_record_prefix(self):
         expected = DataFrame([[1], [2]], columns=['Prefix.0'])
         tm.assert_frame_equal(result, expected)
 
+    def test_nested_object_record_path(self):
+        # GH 22706
+        data = {'state': 'Florida',
+                'info': {
+                    'governor': 'Rick Scott',
+                    'counties': [{'name': 'Dade', 'population': 12345},
+                                 {'name': 'Broward', 'population': 40000},
+                                 {'name': 'Palm Beach', 'population': 60000}]}}
+        result = json_normalize(data, record_path=["info", "counties"])
+        expected = DataFrame([['Dade', 12345],
+                              ['Broward', 40000],
+                              ['Palm Beach', 60000]],
+                             columns=['name', 'population'])
+        tm.assert_frame_equal(result, expected)
+
     def test_more_deeply_nested(self, deep_nested):
 
         result = json_normalize(deep_nested, ['states', 'cities'],


Code review comments:
diff_hunk: @@ -224,6 +224,44 @@ def _pull_field(js, spec):
         sep = str(sep)
     meta_keys = [sep.join(val) for val in meta]
 
+    def _extract(obj, field, seen_meta, level):
+        """
+        Extract field from obj and update `records`, `lengths`, `meta_vals`.
+
+        Parameters
+        ----------
+        obj : dict
+            Unserialized dict
+        field : str
+            The field to extract from obj
+        seen_meta : dict
+            The dict of meta values that have been visited
+        level: int
+            The current level
+        """
+        recs = _pull_field(obj, field)
+
+        # For repeating the metadata later
+        lengths.append(len(recs))
+
+        for val, key in zip(meta, meta_keys):
path: pandas/io/json/normalize.py
text: @user1:
can you make this  immutable function, this is very hard to reason about now

@author:
`_extract` is a new helper function. The original function `_recursive_extract` (and now the caller of `_extract`) is not immutable (it also updates `records`, `lengths`, `meta_vals`).
What I did was to delegate the updates in `_recursive_extract` to `_extract`.

It's strange that we make `_extract` immutable but `_recursive_extract` is not. 
In this PR I want to make minimal changes. Refactoring existing code is certainly not in scope of this PR.

@user1:
@author if you are going to move it, then i'd like to fix it. its not a problem to also fix ``_recursive_extract``.

@user2:
Agreed with @user1 here; would rather see a change that addresses some of the mutability concerns with `recursive_extract` rather than creating a new function 

@author:
I refactor `_recursive_extract` to make it return a tuple of `records` and `meta_vals` (`lengths` was not necessary because it could be computed from `records`).