API: more consistent error message for MultiIndex.from_arrays

[simonjayhawkins]

improve error messages: xref #23922 (comment)

a small change to start. @jorisvandenbossche: do we have/need a master tracker/discussion issue for improving error messages?

[WillAyd]

Hmm what was this doing before if not raising this message already?

[simonjayhawkins]

it raises TypeError: object of type 'int' has no len() if it's an int

[codecov]

Codecov Report

Merging #25189 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #25189      +/-   ##
==========================================
+ Coverage   92.34%   92.34%   +<.01%     
==========================================
  Files         166      166              
  Lines       52310    52316       +6     
==========================================
+ Hits        48304    48310       +6     
  Misses       4006     4006

Flag	Coverage Δ
#multiple	90.78% <100%> (ø)	⬆️
#single	42.84% <50%> (ø)	⬆️

Impacted Files	Coverage Δ
pandas/core/indexes/multi.py	95.62% <100%> (+0.02%)	⬆️

---

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 09633b8...0974cee. Read the comment docs.

[codecov]

Codecov Report

Merging #25189 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #25189      +/-   ##
==========================================
+ Coverage   91.73%   91.73%   +<.01%     
==========================================
  Files         173      173              
  Lines       52841    52845       +4     
==========================================
+ Hits        48476    48480       +4     
  Misses       4365     4365

Flag	Coverage Δ
#multiple	90.3% <100%> (ø)	⬆️
#single	41.72% <60%> (ø)	⬆️

Impacted Files	Coverage Δ
pandas/core/indexes/multi.py	95.62% <100%> (+0.01%)	⬆️

---

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 13a505d...403789a. Read the comment docs.

[simonjayhawkins]

we could replace Input must be a list / sequence of array-likes with Input must be a list / sequence of equal length array-likes and consolidate with the all arrays must be same length message.

..but that could be considered reducing the helpfulness of the messaging.

[WillAyd]

I don’t think you necessarily need to consolidate. Why not have different error messages?

…

Sent from my iPhone

On Feb 6, 2019, at 7:56 AM, Simon Hawkins ***@***.***> wrote: we could replace Input must be a list / sequence of array-likes with Input must be a list / sequence of equal length array-likes and consolidate with the all arrays must be same length message. ..but that could be considered reducing the helpfulness of the messaging. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[simonjayhawkins]

There's helpfulness and handholding.

the api reference states what the input should be. the error message should probably just regurgitate this.

arrays : list / sequence of array-likes

a single error message for each incorrect input would probably be more consistent than many variations.

[simonjayhawkins]

regarding the all arrays must be same length message, xref #13599, which is effectively an invalid input the same as Input must be a list / sequence of array-likes, this perhaps provides an opportunity to improve the docstring and perhaps merge test_from_arrays_invalid_input and test_from_arrays_different_lengths.

however, i've slightly sidetracked from the original change which changes

>>> from pandas import MultiIndex
>>> MultiIndex.from_arrays([1])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\simon\OneDrive\code\pandas-simonjayhawkins\pandas\core\indexes\multi.py", line 340, in from_arrays
    codes, levels = _factorize_from_iterables(arrays)
  File "C:\Users\simon\OneDrive\code\pandas-simonjayhawkins\pandas\core\arrays\categorical.py", line 2710, in _factorize_from_iterables
    return map(list, lzip(*[_factorize_from_iterable(it) for it in iterables]))
  File "C:\Users\simon\OneDrive\code\pandas-simonjayhawkins\pandas\core\arrays\categorical.py", line 2710, in <listcomp>
    return map(list, lzip(*[_factorize_from_iterable(it) for it in iterables]))
  File "C:\Users\simon\OneDrive\code\pandas-simonjayhawkins\pandas\core\arrays\categorical.py", line 2671, in _factorize_from_iterable
    raise TypeError("Input must be list-like")
TypeError: Input must be list-like
>>>

to be more meaningful. [1] is list-like!

and changes..

>>> MultiIndex.from_arrays([[1],1], sortorder=1, names=1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\simon\OneDrive\code\pandas-simonjayhawkins\pandas\core\indexes\multi.py", line 335, in from_arrays
    if len(arrays[i]) != len(arrays[i - 1]):
TypeError: object of type 'int' has no len()
>>>
>>>

to be more meaningful and much more helpful. which int have i got wrong?

both these cases will raise TypeError: Input must be a list / sequence of array-likes with this change.

[jreback]

I recall this exact change from another issue, can you see if you can find it (search for MultiIndex issues).

[jreback]

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

[simonjayhawkins]

@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.

[simonjayhawkins]

I recall this exact change from another issue, can you see if you can find it (search for MultiIndex issues).

#18434 and subsequent PR #18440 is along the same theme and prevents a TypeError: object of type 'zip' has no len().

Here, we are catching the len issue on elements of the list / sequence which should also be list-like.

[jorisvandenbossche]

This is certainly an improvement for the case where you pass a single list/array !

Small comment about the actual message, but also don't have the good answer though.

[jorisvandenbossche]

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.

[simonjayhawkins]

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.

[simonjayhawkins]

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.

[jreback]

lgtm. @jorisvandenbossche were you ok on this?

[WillAyd]

lgtm

[WillAyd]

@simonjayhawkins can you merge master?

@jorisvandenbossche unless you have objections planning to merge after above

[jorisvandenbossche]

Yes, sure!

[jreback]

lgtm. can merge on green after merging master.

[simonjayhawkins]

can merge on green after merging master

@jreback: merged master.

[WillAyd]

Thanks @simonjayhawkins !