Additional tests for ufunc(Series) #26951
Conversation
This adds a set of tests for ufuncs on Series. The goal is to establish the correct behavior prior to implementing `Series.__array_ufunc__`. There are two kinds of xfails right now 1. Series[Sparse] fails because `Series.__array_ufunc__` doesn't yet dispatch to `Series.array.__array_ufunc__` 2. `ufunc(series, series)` when the two series are unaligned. It's been determined that these should align, but isn't currently implemented.
TomAugspurger
added
Numeric Operations
|
Should DTA[tz] be included along with Sparse? |
Codecov Report
@@ Coverage Diff @@
## master #26951 +/- ##
==========================================
+ Coverage 91.98% 91.98% +<.01%
==========================================
Files 180 180
Lines 50772 50774 +2
==========================================
+ Hits 46704 46706 +2
Misses 4068 4068
Continue to review full report at Codecov.
|
IMO, the intention of these tests is to verify the correctness of the future |
|
OK, many of the DTA[tz] ufuncs are currently broken, so its OK by me if you want to leave them out. |
|
0b1e745 adds another xfail for In [1]: import pandas as pd; import numpy as np
In [2]: a = pd.Series([1, 2])
In [3]: b = pd.Index([1, 2])
In [4]: np.add(b, a)
Out[4]: Int64Index([2, 4], dtype='int64')I think that's a bug. It's inconsistent with the binop In [5]: b + a
Out[5]:
0 2
1 4
dtype: int64So the rule is that |
(cherry picked from commit ba5cb55)
|
775c2ef fixes an issue with one of the tests. The test wasn't correct for I think this PR should be ready to go. I'd like to merge Joris' |
pandas/tests/series/test_ufunc.py
Outdated
| def test_binary_ufunc(ufunc, sparse, shuffle, box_other, | ||
| flip, | ||
| arrays_for_binary_ufunc): | ||
| # Check the invariant that |
There was a problem hiding this comment.
can you give a little expl of what the parametizations do if not obvious, e.g. flip & shuffle actually are not immediately obvious
There was a problem hiding this comment.
I may split those out to separate tests. It'll be a bit more code, but much clearer.
| # Check the invariant that | ||
| # ufunc(Series(a), Series(b)) == Series(ufunc(a, b)) | ||
| # with alignment. | ||
| a1, a2 = arrays_for_binary_ufunc |
There was a problem hiding this comment.
can you call these: left_array, right_array
I know its a bit longer, but more readable IMHO
pandas/tests/series/test_ufunc.py
Outdated
| a2 = pd.SparseArray(a2, dtype=pd.SparseDtype('int', 0)) | ||
|
|
||
| name = "name" | ||
| s1 = pd.Series(a1, name=name) |
There was a problem hiding this comment.
left_series and right_series
pandas/tests/series/test_ufunc.py
Outdated
| a2 = a2.take(idx) | ||
|
|
||
| a, b = s1, s2 | ||
| c, d = a1, a2 |
There was a problem hiding this comment.
I find this very confusing, can you use the same terms and not use a, b, c, d?
pandas/tests/series/test_ufunc.py
Outdated
| series = pd.Series(array, name="name") | ||
|
|
||
| a, b = series, other | ||
| c, d = array, other |
There was a problem hiding this comment.
same as above if you can make this more clear
| array = pd.SparseArray(array) | ||
|
|
||
| series = pd.Series(array, name="name") | ||
| result = np.modf(series) |
There was a problem hiding this comment.
ok I c you are doing this here
|
do we have an issue for this? (or maybe just xref the actual ufunc issue at the top of the PR) |
|
Changes
|
|
lgtm. |
|
thanks @TomAugspurger |
This adds a set of tests for ufuncs on Series. The goal is to
establish the correct behavior prior to implementing
Series.__array_ufunc__.There are two kinds of xfails right now
Series.__array_ufunc__doesn'tyet dispatch to
Series.array.__array_ufunc__ufunc(series, series)when the two series are unaligned. It'sbeen determined that these should align, but isn't currently
implemented.
LMK if there's anything I can do to make these tests clearer. The heavy parameterization is a bit unfortunate, but I think the best option.
xref #23293