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: in Python3 MultiIndex.from_tuples cannot take "zipped" tuples #18440

Merged
merged 2 commits into from
Nov 25, 2017

Conversation

Xbar
Copy link
Contributor

@Xbar Xbar commented Nov 23, 2017

MultiIndex.from_tuples accept zipped tuples in python 3. Ensures compatibility
between 2 and 3.

@codecov
Copy link

codecov bot commented Nov 23, 2017

Codecov Report

Merging #18440 into master will decrease coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18440      +/-   ##
==========================================
- Coverage   91.36%   91.32%   -0.04%     
==========================================
  Files         163      163              
  Lines       49700    49764      +64     
==========================================
+ Hits        45407    45446      +39     
- Misses       4293     4318      +25
Flag Coverage Δ
#multiple 89.12% <100%> (-0.03%) ⬇️
#single 40.71% <50%> (+0.99%) ⬆️
Impacted Files Coverage Δ
pandas/core/indexes/multi.py 96.44% <100%> (+0.03%) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/io/clipboard/__init__.py 56.86% <0%> (-19.33%) ⬇️
pandas/core/dtypes/missing.py 90.24% <0%> (-0.5%) ⬇️
pandas/core/indexes/datetimelike.py 96.91% <0%> (-0.2%) ⬇️
pandas/core/frame.py 97.8% <0%> (-0.1%) ⬇️
pandas/core/series.py 94.76% <0%> (-0.08%) ⬇️
pandas/core/dtypes/cast.py 88.52% <0%> (-0.08%) ⬇️
pandas/core/indexes/base.py 96.4% <0%> (-0.04%) ⬇️
pandas/core/base.py 96.54% <0%> (-0.03%) ⬇️
... and 4 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 aaee541...7a220aa. Read the comment docs.

@@ -1206,6 +1206,12 @@ def from_tuples(cls, tuples, sortorder=None, names=None):
MultiIndex.from_product : Make a MultiIndex from cartesian product
of iterables
"""
if not hasattr(tuples, '__len__'):
Copy link
Contributor

Choose a reason for hiding this comment

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

would just do

if not is_list_like(tuples):
    raise TypeError...
elif is_iterator(tuples):
    tuples = list(tuples)

@@ -1206,6 +1206,12 @@ def from_tuples(cls, tuples, sortorder=None, names=None):
MultiIndex.from_product : Make a MultiIndex from cartesian product
of iterables
"""
if not hasattr(tuples, '__len__'):
try:
iter(tuples) # if iterable, then it's iterator or generator
Copy link
Contributor

Choose a reason for hiding this comment

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

needs checking in from_product and from_arrays as well.

needs tests that hit all 3 cases

@jreback jreback changed the title Fix issue 18434 BUG: in Python3 MultiIndex.from_tuples cannot take "zipped" tuples Nov 23, 2017
@jreback jreback added 2/3 Compat Reshaping Concat, Merge/Join, Stack/Unstack, Explode Error Reporting Incorrect or improved errors from pandas labels Nov 23, 2017
@Xbar
Copy link
Contributor Author

Xbar commented Nov 24, 2017

Added the test cases and fix from_tuples/from_arrays/from_product

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.

some error context comments. otherwise lgtm. ping on green.

@@ -1162,6 +1162,11 @@ def from_arrays(cls, arrays, sortorder=None, names=None):
MultiIndex.from_product : Make a MultiIndex from cartesian product
of iterables
"""
if not is_list_like(arrays):
raise TypeError("Inappropriate input for list of arrays.")
Copy link
Contributor

Choose a reason for hiding this comment

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

make a more user-friendly message, maybe

input must be a list-like of arrays

@@ -1206,6 +1211,11 @@ def from_tuples(cls, tuples, sortorder=None, names=None):
MultiIndex.from_product : Make a MultiIndex from cartesian product
of iterables
"""
if not is_list_like(tuples):
raise TypeError('Inappropriate input for tuples.')
Copy link
Contributor

Choose a reason for hiding this comment

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

same (except say tuples)

@@ -1260,6 +1270,11 @@ def from_product(cls, iterables, sortorder=None, names=None):
from pandas.core.categorical import _factorize_from_iterables
from pandas.core.reshape.util import cartesian_product

if not is_list_like(iterables):
Copy link
Contributor

Choose a reason for hiding this comment

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

same

result = MultiIndex.from_arrays(arrays)
assert list(result) == list(self.index)
# iterator as input
Copy link
Contributor

Choose a reason for hiding this comment

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

blank line here

# iterator as input
result2 = MultiIndex.from_arrays(iter(arrays))
assert list(result2) == list(self.index)
# invlide iterator input
Copy link
Contributor

Choose a reason for hiding this comment

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

same

expected = MultiIndex(levels=[[1, 3], [2, 4]],
labels=[[0, 1], [0, 1]],
names=['a', 'b'])
# input tuples
Copy link
Contributor

Choose a reason for hiding this comment

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

same

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.

pls rebase as well. lgtm. ping on green.

@@ -827,6 +837,16 @@ def test_from_product(self):
tm.assert_index_equal(result, expected)
assert result.names == names

# iterator as input
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 make this a separate test (the added code), and do so for from_product, from_tuples, from_arrays as well, call it something like

test_multiindex_from_product_iterable and add the issue number to it.

@@ -1206,6 +1211,11 @@ def from_tuples(cls, tuples, sortorder=None, names=None):
MultiIndex.from_product : Make a MultiIndex from cartesian product
of iterables
"""
if not is_list_like(tuples):
raise TypeError('Input must be a list /sequence of tuple-likes.')
Copy link
Contributor

Choose a reason for hiding this comment

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

need a space after the /

labels=[[0, 1], [0, 1]],
names=['a', 'b'])

# input tuples
Copy link
Contributor

Choose a reason for hiding this comment

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

same (new test)

names=['a', 'b'])

# input tuples
res1 = MultiIndex.from_tuples(((1, 2), (3, 4)), names=['a', 'b'])
Copy link
Contributor

Choose a reason for hiding this comment

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

use result =

assert res1.equals(expected)

# input iterator for tuples
res2 = MultiIndex.from_tuples(zip([1, 3], [2, 4]), names=['a', 'b'])
Copy link
Contributor

Choose a reason for hiding this comment

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

same

# input iterator for tuples
res2 = MultiIndex.from_tuples(zip([1, 3], [2, 4]), names=['a', 'b'])
assert expected.names == res2.names
assert res2.equals(expected)
Copy link
Contributor

Choose a reason for hiding this comment

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

use tm.assert_index_equal(result, expected) for the checking

result = MultiIndex.from_arrays(arrays)
assert list(result) == list(self.index)

# iterator as input
Copy link
Contributor

Choose a reason for hiding this comment

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

separate test

for lev, lab in zip(self.levels, self.labels):
for idx, (lev, lab) in enumerate(zip(self.levels, self.labels)):
na_idxs = np.where(lab == -1)[0]

Copy link
Contributor

Choose a reason for hiding this comment

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

looks like you picked up another commit.

@Xbar
Copy link
Contributor Author

Xbar commented Nov 25, 2017

All build passed.


# iterator as input
result = MultiIndex.from_arrays(iter(arrays))
assert list(result) == list(self.index)
Copy link
Contributor

Choose a reason for hiding this comment

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

use tm.assert_index_equal, IOW construct the expected MI


# iterator as input
result = MultiIndex.from_product(iter([first, second]), names=names)
assert result.equals(expected)
Copy link
Contributor

Choose a reason for hiding this comment

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

use assert_index_equal


# input tuples
result = MultiIndex.from_tuples(((1, 2), (3, 4)), names=['a', 'b'])
assert expected.names == result.names
Copy link
Contributor

Choose a reason for hiding this comment

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

same

# input tuples
result = MultiIndex.from_tuples(((1, 2), (3, 4)), names=['a', 'b'])
assert expected.names == result.names
assert result.equals(expected)
Copy link
Contributor

Choose a reason for hiding this comment

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

same

names=['a', 'b'])

result = MultiIndex.from_tuples(zip([1, 3], [2, 4]), names=['a', 'b'])
assert expected.names == result.names
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@jreback jreback added this to the 0.22.0 milestone Nov 25, 2017
@jreback jreback merged commit 0bcd77e into pandas-dev:master Nov 25, 2017
@jreback
Copy link
Contributor

jreback commented Nov 25, 2017

thanks @Xbar nice patch and very responsive interactions!

@Xbar
Copy link
Contributor Author

Xbar commented Nov 25, 2017

Thanks! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Error Reporting Incorrect or improved errors from pandas Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: in Python3 MultiIndex.from_tuples cannot take "zipped" tuples
2 participants