Skip to content
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.

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

BUG: Index constructor support tupleization for mixed levels #18514

Merged

Conversation

toobaz
Copy link
Member

@toobaz toobaz commented Nov 27, 2017

@toobaz toobaz force-pushed the index_tupleize_mixed_level branch 2 times, most recently from 22234f0 to 39684cb Compare November 27, 2017 09:59
@codecov
Copy link

codecov bot commented Nov 27, 2017

Codecov Report

Merging #18514 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18514      +/-   ##
==========================================
- Coverage   91.35%   91.33%   -0.02%     
==========================================
  Files         164      164              
  Lines       49801    49799       -2     
==========================================
- Hits        45494    45484      -10     
- Misses       4307     4315       +8
Flag Coverage Δ
#multiple 89.13% <100%> (ø) ⬆️
#single 40.81% <50%> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/base.py 96.47% <100%> (+0.04%) ⬆️
pandas/core/base.py 96.55% <100%> (ø) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.81% <0%> (-0.1%) ⬇️

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 88ab693...ea824a3. Read the comment docs.

@jreback jreback added Bug Dtype Conversions Unexpected or buggy dtype conversions labels Nov 27, 2017
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

lgtm. some comments. reducing code is always +1 from me!

@@ -147,6 +147,7 @@ Indexing
- Bug in :func:`DataFrame.groupby` where tuples were interpreted as lists of keys rather than as keys (:issue:`17979`, :issue:`18249`)
- Bug in :func:`MultiIndex.remove_unused_levels`` which would fill nan values (:issue:`18417`)
- Bug in :func:`MultiIndex.from_tuples`` which would fail to take zipped tuples in python3 (:issue:`18434`)
- Bug in :class:`Index`` initialization from list of mixed type tuples (:issue:`18505`)
Copy link
Contributor

Choose a reason for hiding this comment

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

initialization -> construction

@@ -106,6 +106,16 @@ def test_construction_list_mixed_tuples(self):
assert isinstance(idx2, Index)
assert not isinstance(idx2, MultiIndex)

Copy link
Contributor

Choose a reason for hiding this comment

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

make this is a new test.

parametrize on input types (iterator, tuple, list) & (np.nan, None) for a missing value

Copy link
Member Author

Choose a reason for hiding this comment

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

(done)

@jreback ping

@toobaz toobaz force-pushed the index_tupleize_mixed_level branch from 39684cb to e4d8f89 Compare November 27, 2017 11:50
# GH 18505 : valid tuples containing NaN
values = [(1, 'two'), (3., na_value)]
idx = Index(vtype(values))
assert isinstance(idx, MultiIndex)
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 construct the actual index and use assert_index_equal here

@toobaz toobaz force-pushed the index_tupleize_mixed_level branch from e4d8f89 to ae005cb Compare November 27, 2017 23:14
@toobaz
Copy link
Member Author

toobaz commented Nov 28, 2017

@jreback ping

@jorisvandenbossche
Copy link
Member

Do you know why the try except is no longer needed? Or was it not covered anyhow?

@toobaz
Copy link
Member Author

toobaz commented Nov 28, 2017

Do you know why the try except is no longer needed?

Not really... apparently, at some moment mixed levels weren't supported.

@jorisvandenbossche
Copy link
Member

And codecov also indicates the tests never reached the except block, so probably indeed fixed in from_tuples

@jorisvandenbossche
Copy link
Member

Can you add a test case for the original series issue as well?

@toobaz
Copy link
Member Author

toobaz commented Nov 28, 2017

Can you add a test case for the original series issue as well?

Done... but it xfails due to #18480

@jorisvandenbossche jorisvandenbossche added this to the 0.22.0 milestone Nov 28, 2017
@jorisvandenbossche jorisvandenbossche changed the title BUG: support tupleization for mixed levels BUG: Index constructor support tupleization for mixed levels Nov 28, 2017
@jorisvandenbossche jorisvandenbossche merged commit 7463f86 into pandas-dev:master Nov 28, 2017
@jorisvandenbossche
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Dtype Conversions Unexpected or buggy dtype conversions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating series with multiindex from dict with tuple keys with None
3 participants