BUG: pd.read_json May Not Maintain Numeric String Index #38727

theoniko · 2020-12-27T18:29:22Z

closes pd.read_json May Not Maintain Numeric String Index #28556
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

arw2019

Thanks @theoniko for the PR!

Some comments. We'll also need a whatsnew for this (1.3)

arw2019 · 2020-12-27T18:36:07Z

pandas/io/json/_json.py

+                and obj._get_axis(axis_name).dtype == "object"
+                and obj.index.inferred_type == "string"
+            ):
+                index_list = list(obj._get_axis(axis_name).values)


do we have to convert to a list here? We should be able to inspect values directly I think

arw2019 · 2020-12-27T18:37:22Z

pandas/io/json/_json.py

+                and obj.index.inferred_type == "string"
+            ):
+                index_list = list(obj._get_axis(axis_name).values)
+                are_all_strings = all(type(element) == str for element in index_list)


we have helpers got these kinds of checks in pandas/core/dtypes/common.py, may as well use those

arw2019 · 2020-12-27T18:38:48Z

pandas/io/json/_json.py

+                are_all_int_or_floats = all(
+                    are_int_or_floats is True for element in are_int_or_floats
+                )
+                if are_all_int_or_floats and are_all_strings:


is this ever hit? only one of those can be true at any time, no?

arw2019 · 2020-12-27T18:39:05Z

pandas/io/json/_json.py

                    result = True
                except (TypeError, ValueError):
                    pass
-


pls revert this to minimize the diff

arw2019 · 2020-12-27T18:39:29Z

pandas/tests/io/json/test_pandas.py

        # JSON objects. JSON keys are by definition strings, so there's no way
        # to disambiguate whether those keys actually were strings or numeric
        # beforehand and numeric wins out.
        # TODO: Split should be able to support this


can we delete this TODO now?

mroeschke · 2020-12-27T19:32:41Z

pandas/io/json/_json.py

        obj = self.obj
        assert obj is not None  # for mypy
        for axis_name in obj._AXIS_ORDERS:
+            if (


This if condition seems too specific. Is there a more generic way to handle this?

As far as i understood the minimum preconditions to keep string index should in read_json orient equal to split or index and the index of json format to be a list of int or float strings. Am i right? Should i move the checks in _try_convert_data?
Do you have a more conrete and robust suggestion?

I tried a slightly different way. Could you please re-review?

jreback

needs to be less specialized (as indicated by comments)

pep8speaks · 2020-12-29T12:20:57Z

Hello @theoniko! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-01-04 22:32:49 UTC

WillAyd · 2020-12-29T18:34:04Z

pandas/tests/io/json/test_pandas.py

            expected.columns = expected.columns.astype(np.int64)
+        elif convert_axes and orient == "split":
+            expected.columns = expected.columns.astype(np.int64)
+            expected.index = expected.index.astype(str)


Do we need this conversion? I think the problem is that we are converting when we don't need to

theoniko · 2021-01-01T09:57:33Z

All requested changes were made. Please re-review the pr.

jreback · 2021-01-01T20:56:55Z

doc/source/whatsnew/v1.3.0.rst

  for subclasses of ``DataFrame`` or ``Series`` (:issue:`33748`).
 - Bug in :func:`json_normalize` resulting in the first element of a generator object not being included in the returned ``DataFrame`` (:issue:`35923`)
-
+- Bug in :func:`read_json` does not maintan numeric string index (:issue:`28556`)


this is not specific enough to the actual case, pls update

jreback · 2021-01-01T20:58:01Z

pandas/io/json/_json.py

+                and self.orient == "split"
+                and len(data)
+            ):
+                result = False


instead of using result at all, just return on a conversion

I moved the condition to the end and removed the result variable.

jreback · 2021-01-01T21:01:44Z

pandas/io/json/_json.py

        if data.dtype == "object":
-
+            if (
+                isinstance(data, Index)


i don't think you need to test if an Index

jreback · 2021-01-01T21:02:10Z

merge master as well

jreback · 2021-01-03T16:51:16Z

pandas/io/json/_json.py

                pass

-        return data, result
+        if name == "index" and self.orient == "split" and len(data):


ok looks a lot better. can you add a comment here and structure this more like

# if we have an index, we want to preserve dtypes if name == "index" and len(data): if self.orient == "split": return data, False return data, True

jreback · 2021-01-03T21:07:26Z

doc/source/whatsnew/v1.3.0.rst

 - Bug in :func:`json_normalize` resulting in the first element of a generator object not being included in the returned ``DataFrame`` (:issue:`35923`)
 - Bug in :func:`read_excel` forward filling :class:`MultiIndex` names with multiple header and index columns specified (:issue:`34673`)
 - :func:`pandas.read_excel` now respects :func:``pandas.set_option`` (:issue:`34252`)
+- Bug in :func:`read_json` when ``orient="split"`` does not maintan numeric string index (:issue:`28556`)


Bug in :func:read_json when orient="split" does not maintain numeric string index (:issue:28556)

doc/source/whatsnew/v1.3.0.rst

jreback · 2021-01-04T22:33:07Z

lgtm pushed a small doc change and can merge on green

…8727)

arw2019 suggested changes Dec 27, 2020

View reviewed changes

mroeschke reviewed Dec 27, 2020

View reviewed changes

rhshadrach added Bug Dtype Conversions Unexpected or buggy dtype conversions IO JSON read_json, to_json, json_normalize labels Dec 28, 2020

theoniko force-pushed the read-json-numeric-string-index branch from dd46945 to 42f912e Compare December 28, 2020 09:57

jreback requested changes Dec 29, 2020

View reviewed changes

theoniko force-pushed the read-json-numeric-string-index branch 2 times, most recently from 28b31fb to bb21639 Compare December 29, 2020 12:20

theoniko requested a review from mroeschke December 29, 2020 12:21

theoniko force-pushed the read-json-numeric-string-index branch 4 times, most recently from 01488e4 to bfd8e89 Compare December 29, 2020 13:26

WillAyd reviewed Dec 29, 2020

View reviewed changes

theoniko force-pushed the read-json-numeric-string-index branch from bfd8e89 to db10b73 Compare January 1, 2021 09:08

jreback requested changes Jan 1, 2021

View reviewed changes

theoniko force-pushed the read-json-numeric-string-index branch 6 times, most recently from 3ba5e89 to 0762111 Compare January 2, 2021 13:17

theoniko requested a review from jreback January 2, 2021 16:24

jreback requested changes Jan 3, 2021

View reviewed changes

theoniko force-pushed the read-json-numeric-string-index branch 2 times, most recently from 9234406 to 2e5e743 Compare January 3, 2021 17:33

jreback reviewed Jan 3, 2021

View reviewed changes

BUG: pd.read_json May Not Maintain Numeric String Index

f2a2785

theoniko force-pushed the read-json-numeric-string-index branch from 2e5e743 to f2a2785 Compare January 4, 2021 18:11

jreback approved these changes Jan 4, 2021

View reviewed changes

jreback added this to the 1.3 milestone Jan 4, 2021

jreback reviewed Jan 4, 2021

View reviewed changes

doc/source/whatsnew/v1.3.0.rst Outdated Show resolved Hide resolved

Update doc/source/whatsnew/v1.3.0.rst

58ad3d7

jreback merged commit 9bc1364 into pandas-dev:master Jan 4, 2021

luckyvs1 pushed a commit to luckyvs1/pandas that referenced this pull request Jan 20, 2021

BUG: pd.read_json May Not Maintain Numeric String Index (pandas-dev#3…

11b0db1

…8727)

Uh oh!

BUG: pd.read_json May Not Maintain Numeric String Index #38727

BUG: pd.read_json May Not Maintain Numeric String Index #38727

Uh oh!

Conversation

theoniko commented Dec 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arw2019 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!

jreback left a comment

Choose a reason for hiding this comment

Uh oh!

pep8speaks commented Dec 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2021-01-04 22:32:49 UTC

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theoniko commented Jan 1, 2021

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!

jreback commented Jan 1, 2021

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!

Uh oh!

jreback commented Jan 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

theoniko commented Dec 27, 2020 •

edited

Loading

pep8speaks commented Dec 29, 2020 •

edited

Loading