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.

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

API: more consistent error message for MultiIndex.from_arrays #25189

Merged
merged 7 commits into from
Feb 20, 2019
Merged

API: more consistent error message for MultiIndex.from_arrays #25189

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0974cee
more consistent error message for MultiIndex.from_arrays
simonjayhawkins Feb 6, 2019
06cb448
add additional test case for loop check
simonjayhawkins Feb 6, 2019
1438459
own loop for sequence element list-like check
simonjayhawkins Feb 6, 2019
84993d7
test with tuples for invalid input
simonjayhawkins Feb 8, 2019
0974c3b
test with tuples for valid input
simonjayhawkins Feb 8, 2019
96eeed2
Merge remote-tracking branch 'upstream/master' into mi-constructor-error
simonjayhawkins Feb 8, 2019
403789a
Merge remote-tracking branch 'upstream/master' into mi-constructor-error
simonjayhawkins Feb 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion pandas/core/indexes/multi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,11 +324,17 @@ def from_arrays(cls, arrays, sortorder=None, names=None):
codes=[[0, 0, 1, 1], [1, 0, 1, 0]],
names=['number', 'color'])
"""
error_msg = "Input must be a list / sequence of array-likes."
Copy link
Member

Choose a reason for hiding this comment

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

We could maybe think about how to improve the actual message as well, because on a first read I was interpreting this as "Input must be [a list] or [a sequence of array-likes]" (while of course it is "[list or sequence] of array-likes"), which confused me at first ..

To be true to the code, what it actually needs to be is a "list-like of list-likes"? Which is also not that nice to write ..
I am wondering if a more strict error message (stricter than what we allow), something like "Input must be a list of arrays" is not actually easier to understand for users.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was also thinking that the messages should be changed from Input must be... to something along the lines of 'arrays' parameter of MultiIndex.from_arrays must be... and then regurgitate whatever is in the docstring.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am wondering if a more strict error message (stricter than what we allow), something like "Input must be a list of arrays" is not actually easier to understand for users.

IIUC the reason that a sequence is accepted is to provide backward compatibility with zip. So sequence does not necessarily need to be mentioned in the docstring.

if not is_list_like(arrays):
raise TypeError("Input must be a list / sequence of array-likes.")
raise TypeError(error_msg)
elif is_iterator(arrays):
arrays = list(arrays)

# Check if elements of array are list-like
Copy link
Contributor

Choose a reason for hiding this comment

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

do we fully test this? (test with tuples as well)

Copy link
Member Author

Choose a reason for hiding this comment

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

@jreback: not fully. i've added a test that is basically a cut and paste from another test. could parameterise now or refactor in a follow-on PR. i prefer the later since some other refactoring of the tests may be possible and may detract from the current change.

for array in arrays:
if not is_list_like(array):
raise TypeError(error_msg)

# Check if lengths of all arrays are equal or not,
# raise ValueError, if not
for i in range(1, len(arrays)):
Expand Down
6 changes: 2 additions & 4 deletions pandas/tests/indexes/multi/test_constructor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,9 @@ def test_from_arrays_empty():


@pytest.mark.parametrize('invalid_sequence_of_arrays', [
1, [1], [1, 2], [[1], 2], 'a', ['a'], ['a', 'b'], [['a'], 'b']])
1, [1], [1, 2], [[1], 2], [1, [2]], 'a', ['a'], ['a', 'b'], [['a'], 'b']])
def test_from_arrays_invalid_input(invalid_sequence_of_arrays):
msg = (r"Input must be a list / sequence of array-likes|"
r"Input must be list-like|"
r"object of type 'int' has no len\(\)")
msg = "Input must be a list / sequence of array-likes"
with pytest.raises(TypeError, match=msg):
MultiIndex.from_arrays(arrays=invalid_sequence_of_arrays)

Expand Down