Skip to content

Fixed issue with read_json and partially missing MI names #19177

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 16, 2018
Merged

Fixed issue with read_json and partially missing MI names #19177

merged 1 commit into from
Jan 16, 2018

Conversation

WillAyd
Copy link
Member

@WillAyd WillAyd commented Jan 11, 2018

jreback
jreback requested changes Jan 12, 2018
View reviewed changes
doc/source/io.rst Outdated
name.
Please note that the literal string 'index' as the name of an ``Index`` is not
round-trippable, nor are any names beginning with 'level_' within a
``MultiIndex``. These are used by default in ``write_json`` to indicate missing
Copy link
Contributor

Choose a reason for hiding this comment

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

can you use :class`Index` and :func:`DataFrame.to_json` (you mean that instead of write_json I think)

pandas/tests/io/json/test_json_table_schema.py Outdated
names=('foo', 'level_1'))
])
def test_warns_non_roundtrippable_names(self, idx):
df = pd.DataFrame([[]], index=idx)
Copy link
Contributor

Choose a reason for hiding this comment

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

add the issue number here

@jreback jreback added Indexing Related to indexing on series/frames, not to indexes themselves IO JSON read_json, to_json, json_normalize MultiIndex labels Jan 12, 2018
@WillAyd WillAyd force-pushed the json-missing-index branch from 95b2a99 to 1306b0e Compare January 12, 2018 23:14
@codecov
Copy link

codecov bot commented Jan 12, 2018

Codecov Report

Merging #19177 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #19177      +/-   ##
==========================================
- Coverage   91.53%   91.52%   -0.01%     
==========================================
  Files         147      148       +1     
  Lines       48797    48784      -13     
==========================================
- Hits        44664    44648      -16     
- Misses       4133     4136       +3
Flag Coverage Δ
#multiple 89.89% <100%> (-0.01%) ⬇️
#single 41.6% <10%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/io/json/json.py 92.6% <ø> (ø) ⬆️
pandas/io/json/table_schema.py 98.29% <100%> (+0.09%) ⬆️
pandas/core/dtypes/cast.py 88.42% <0%> (-0.24%) ⬇️
pandas/core/ops.py 91.89% <0%> (-0.18%) ⬇️
pandas/core/indexes/interval.py 92.19% <0%> (-0.11%) ⬇️
pandas/core/internals.py 94.42% <0%> (-0.07%) ⬇️
pandas/tseries/offsets.py 97.06% <0%> (-0.03%) ⬇️
pandas/core/frame.py 97.62% <0%> (ø) ⬆️
pandas/core/reshape/reshape.py 100% <0%> (ø) ⬆️
pandas/tools/hashing.py 100% <0%> (ø)
... and 1 more

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 8347ff8...1306b0e. Read the comment docs.

@codecov
Copy link

codecov bot commented Jan 12, 2018 •
edited
Loading

Codecov Report

Merging #19177 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #19177      +/-   ##
==========================================
- Coverage   91.55%   91.53%   -0.03%     
==========================================
  Files         147      147              
  Lines       48812    48818       +6     
==========================================
- Hits        44690    44684       -6     
- Misses       4122     4134      +12
Flag Coverage Δ
#multiple 89.9% <100%> (-0.03%) ⬇️
#single 41.7% <10%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/io/json/json.py 92.6% <ø> (ø) ⬆️
pandas/io/json/table_schema.py 98.29% <100%> (+0.09%) ⬆️
pandas/plotting/_converter.py 65.22% <0%> (-1.74%) ⬇️

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 787ab55...3c9ff1d. Read the comment docs.

@WillAyd WillAyd force-pushed the json-missing-index branch from 1306b0e to 3c9ff1d Compare January 15, 2018 01:51
@jreback jreback added this to the 0.23.0 milestone Jan 16, 2018
jreback
jreback reviewed Jan 16, 2018
View reviewed changes
pandas/io/json/table_schema.py
``Index``, this function sets the name of the returned ``DataFrame`` to
``None`` when said string is encountered. Therefore, intentional usage
of `index` as the ``Index`` name is not supported.
Because :func:`DataFrame.to_json` uses the string 'index' to denote a
Copy link
Contributor

Choose a reason for hiding this comment

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

how much would we break if we used something like what we do in parquet, e.g. __index_level_0__ or something instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @TomAugspurger

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I was thinking about that too. It'd be nice to standardize these at some point, and the sooner the better (doesn't have to be here though. I'll open a new issue).

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah ok makes sense (and actually we could have back compat on this)

Copy link
Contributor

Choose a reason for hiding this comment

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

#19261

@jreback
Copy link
Contributor

jreback commented Jan 16, 2018

lgtm. just asking the question above.

@jreback jreback merged commit c271d4d into pandas-dev:master Jan 16, 2018
@jreback
Copy link
Contributor

jreback commented Jan 16, 2018

thanks @WillAyd

@TomAugspurger will open a new issue to discuss

@WillAyd WillAyd deleted the json-missing-index branch February 27, 2018 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

@TomAugspurger TomAugspurger TomAugspurger left review comments

Assignees
No one assigned
Labels
Indexing Related to indexing on series/frames, not to indexes themselves IO JSON read_json, to_json, json_normalize MultiIndex
Projects
None yet
Milestone
0.23.0
Development

Successfully merging this pull request may close these issues.

read_json and orient='table' Incorrect For MI And Some Missing Values
3 participants
@WillAyd @jreback @TomAugspurger