BUG/TST: Empty input arrays in cartesian_product and MultiIndex (#12258) #14151
Conversation
| @@ -31,14 +31,20 @@ def cartesian_product(X): | |||
| array([1, 2, 1, 2, 1, 2])] | |||
|
|
|||
| """ | |||
| if len(X) == 0: | |||
| return [] | |||
There was a problem hiding this comment.
I like it as it is. Though, maybe this should raise ValueError.
There was a problem hiding this comment.
list like is fine
just should check and raise an error of not
Current coverage is 85.26% (diff: 100%)@@ master #14151 diff @@
==========================================
Files 140 140
Lines 50579 50593 +14
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 43121 43137 +16
+ Misses 7458 7456 -2
Partials 0 0
|
|
|
||
| lenX = np.fromiter((len(x) for x in X), dtype=int) | ||
| cumprodX = np.cumproduct(lenX) | ||
|
|
||
| a = np.roll(cumprodX, 1) | ||
| a[0] = 1 | ||
|
|
||
| b = cumprodX[-1] / cumprodX |
There was a problem hiding this comment.
try/except around this instead is more idomatic
There was a problem hiding this comment.
It doesn't raise:
0/np.array([0,1])
Out[83]: array([ nan, 0.])
/opt/eclipse/plugins/org.python.pydev_5.1.2.201606231256/pysrc/pydevconsole.py:1: RuntimeWarning: invalid value encountered in true_divideHence this strange behaviour in #12258.
jreback
added
MultiIndex
Compat
| @@ -31,14 +31,20 @@ def cartesian_product(X): | |||
| array([1, 2, 1, 2, 1, 2])] | |||
|
|
|||
| """ | |||
There was a problem hiding this comment.
can you add a Parameters/Returns section
There was a problem hiding this comment.
further I think that we require a list-like here (maybe its more strict, in that we require an actual array). can you add some tests?
There was a problem hiding this comment.
The code indicates it should be at least a list-like (or rather a list-like of list-likes). And pandas.compat.product (or itertools.product) is somewhat similar.
I'll fix the docstring and add tests for it. Unless you think we need something stricter.
|
ping when updated / green. |
|
@jreback Sorry for the delay. I'll try to get to it before the weekend. I think I'll have to touch also |
| invalid_inputs = [1, [1], [1, 2], [[1], 2], | ||
| 'a', ['a'], ['a', 'b'], [['a'], 'b']] | ||
| for i in invalid_inputs: | ||
| tm.assertRaises(TypeError, MultiIndex.from_arrays, arrays=i) |
There was a problem hiding this comment.
Probably not the most descriptive test...
TypeError is raised at different places for different inputs. I couldn't quickly think of anything better and clean enough - but can try to improve it if needed.
|
@jreback It took me longer than I expected. |
git diff upstream/master | flake8 --diffThis commit:
cartesian_productwhen some input arrays are empty.from_arraysand.from_product