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

BUG: Series.combine() fails with ExtensionArray inside of Series #21183

Merged
merged 18 commits into from
Jun 8, 2018
Merged

BUG: Series.combine() fails with ExtensionArray inside of Series #21183

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
7469ca9
BUG: Series.combine() fails with ExtensionArray inside of Series
gfyoung May 23, 2018
bbb6640
Merge remote-tracking branch 'upstream/master' into issue20825
Dr-Irv May 23, 2018
339b23a
Fixes as requested by pandas maintainers
Dr-Irv May 23, 2018
61a09e7
Fix PEP8 issues
Dr-Irv May 23, 2018
d862e83
Merge remote-tracking branch 'upstream/master' into issue20825
Dr-Irv May 24, 2018
4c925fc
get rid of is_sequence_of_dtype
Dr-Irv May 24, 2018
27480ac
move whatsnew from 0.23.1 to 0.24
Dr-Irv May 24, 2018
f96372e
Merge remote-tracking branch 'upstream/master' into issue20825
Dr-Irv May 29, 2018
677fe18
remove binop changes and make it work right with scalar
Dr-Irv May 29, 2018
9fceee7
Move test_combine tests to base class
Dr-Irv May 30, 2018
1010cb5
Fix lint issue
Dr-Irv May 31, 2018
aceea9f
Change NotImplemented to NotImplementedError
Dr-Irv May 31, 2018
79506ac
Merge remote-tracking branch 'upstream/master' into issue20825
Dr-Irv May 31, 2018
0e4720b
put in keyword arg for na_value_for_dtype
Dr-Irv Jun 4, 2018
2a21117
Merge remote-tracking branch 'upstream/master' into issue20825
Dr-Irv Jun 6, 2018
e08f832
Merge branch 'master' of github.com:Dr-Irv/pandas into issue20825
Dr-Irv Jun 6, 2018
d3ed2c7
cosmetic changes. fix test_add for category
Dr-Irv Jun 6, 2018
4ca28b2
Doc changes
Dr-Irv Jun 7, 2018
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
33 changes: 33 additions & 0 deletions pandas/tests/extension/base/methods.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,36 @@ def test_factorize_equivalence(self, data_for_grouping, na_sentinel):

tm.assert_numpy_array_equal(l1, l2)
self.assert_extension_array_equal(u1, u2)

def test_combine_le(self, data_repeated):
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 give a 1-liner explaining what this is testing. the name of the test is uninformative.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

# GH 20825
orig_data1, orig_data2 = data_repeated(2)
s1 = pd.Series(orig_data1)
s2 = pd.Series(orig_data2)
result = s1.combine(s2, lambda x1, x2: x1 <= x2)
expected = pd.Series([a <= b for (a, b) in
zip(list(orig_data1), list(orig_data2))])
self.assert_series_equal(result, expected)

val = s1.iloc[0]
result = s1.combine(val, lambda x1, x2: x1 <= x2)
expected = pd.Series([a <= val for a in list(orig_data1)])
self.assert_series_equal(result, expected)

def test_combine_add(self, data_repeated):
# GH 20825
orig_data1, orig_data2 = data_repeated(2)
s1 = pd.Series(orig_data1)
s2 = pd.Series(orig_data2)
result = s1.combine(s2, lambda x1, x2: x1 + x2)
expected = pd.Series(
orig_data1._from_sequence([a + b for (a, b) in
zip(list(orig_data1),
list(orig_data2))]))
self.assert_series_equal(result, expected)

val = s1.iloc[0]
result = s1.combine(val, lambda x1, x2: x1 + x2)
expected = pd.Series(
orig_data1._from_sequence([a + val for a in list(orig_data1)]))
self.assert_series_equal(result, expected)
25 changes: 12 additions & 13 deletions pandas/tests/extension/category/test_categorical.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@

import pytest
import numpy as np
import pandas as pd

import pandas.util.testing as tm

from pandas.api.types import CategoricalDtype
from pandas import Categorical
Expand Down Expand Up @@ -32,6 +29,15 @@ def data_missing():
return Categorical([np.nan, 'A'])


@pytest.fixture
def data_repeated():
"""Return different versions of data for count times"""
def gen(count):
for _ in range(count):
yield Categorical(make_data())
yield gen


@pytest.fixture
def data_for_sorting():
return Categorical(['A', 'B', 'C'], categories=['C', 'A', 'B'],
Expand Down Expand Up @@ -157,16 +163,9 @@ class TestMethods(base.BaseMethodsTests):
def test_value_counts(self, all_data, dropna):
pass

def test_combine(self):
# GH 20825
orig_data1 = make_data()
orig_data2 = make_data()
s1 = pd.Series(Categorical(orig_data1, ordered=True))
s2 = pd.Series(Categorical(orig_data2, ordered=True))
result = s1.combine(s2, lambda x1, x2: x1 <= x2)
expected = pd.Series([a <= b for (a, b) in
zip(orig_data1, orig_data2)])
tm.assert_series_equal(result, expected)
@pytest.mark.skip(reason="combine add for categorical not supported")
def test_combine_add(self, data_repeated):
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

does it raise? if so pls tests that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is interesting. Since combine by definition in the docs does an element by element operation, if you do something like:

In [1]: import pandas as pd                                                     
                                                                                
In [2]: cat1 = pd.Categorical(values=["one","two","three","three","two","one"], 
   ...:  categories=["one","two","three"], ordered=True)                        
                                                                                
In [3]: s1 = pd.Series(cat1)                                                    
                                                                                
In [4]: s1                                                                      
Out[4]:                                                                         
0      one                                                                      
1      two                                                                      
2    three                                                                      
3    three                                                                      
4      two                                                                      
5      one                                                                      
dtype: category                                                                 
Categories (3, object): [one < two < three]                                     
                                                                                
In [5]: s1.combine("abc", lambda x,y : x+y)

In pandas 0.23.0, this fails. But the binary operation is defined on an element by element basis, so the new implementation will return:

Out[4]:
0      oneabc
1      twoabc
2    threeabc
3    threeabc
4      twoabc
5      oneabc
dtype: object

IMHO, this is correct, because of what combine is supposed to do. So I will test that.



class TestCasting(base.BaseCastingTests):
Expand Down
9 changes: 9 additions & 0 deletions pandas/tests/extension/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ def all_data(request, data, data_missing):
return data_missing


@pytest.fixture
def data_repeated():
"""Return different versions of data for count times"""
def gen(count):
for _ in range(count):
yield NotImplemented
Copy link
Member

Choose a reason for hiding this comment

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

NotImplemented -> NotImplementedError

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

yield gen


@pytest.fixture
def data_for_sorting():
"""Length-3 array with a known sort order.
Expand Down
24 changes: 8 additions & 16 deletions pandas/tests/extension/decimal/test_decimal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ def data_missing():
return DecimalArray([decimal.Decimal('NaN'), decimal.Decimal(1)])


@pytest.fixture
def data_repeated():
def gen(count):
for _ in range(count):
yield DecimalArray(make_data())
yield gen


@pytest.fixture
def data_for_sorting():
return DecimalArray([decimal.Decimal('1'),
Expand Down Expand Up @@ -138,22 +146,6 @@ def test_value_counts(self, all_data, dropna):

tm.assert_series_equal(result, expected)

def test_combine(self):
# GH 20825
orig_data1 = make_data()
orig_data2 = make_data()
s1 = pd.Series(DecimalArray(orig_data1))
s2 = pd.Series(DecimalArray(orig_data2))
result = s1.combine(s2, lambda x1, x2: x1 <= x2)
expected = pd.Series([a <= b for (a, b) in
zip(orig_data1, orig_data2)])
tm.assert_series_equal(result, expected)

result = s1.combine(s2, lambda x1, x2: x1 + x2)
expected = pd.Series(DecimalArray([a + b for (a, b) in
zip(orig_data1, orig_data2)]))
tm.assert_series_equal(result, expected)


class TestCasting(BaseDecimal, base.BaseCastingTests):
pass
Expand Down
8 changes: 8 additions & 0 deletions pandas/tests/extension/json/test_json.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,14 @@ def test_sort_values_missing(self, data_missing_for_sorting, ascending):
super(TestMethods, self).test_sort_values_missing(
data_missing_for_sorting, ascending)

@pytest.mark.skip(reason="combine for JSONArray not supported")
def test_combine_le(self, data_repeated):
pass

@pytest.mark.skip(reason="combine for JSONArray not supported")
def test_combine_add(self, data_repeated):
pass


class TestCasting(BaseJSON, base.BaseCastingTests):
@pytest.mark.xfail
Expand Down