Fix for #12723: Unexpected behavior with binary operators and fill…

[facaiy]

• closes #xxxx
• tests added / passed
• passes git diff upstream/master | flake8 --diff
• whatsnew entry

closes #12723

[jreback]

you need to do the eval first, THEN, fill.

[facaiy]

Do you mean that df2.add(2, fill_value=0) should act like df2.add(2).fillna(0), instead of df2.fillna(0).add(2)?

[jreback]

the steps are align, arith then fill (with mask).

align is moot in this case. yes you are right, fill then arith.

In [7]: Series([np.nan,1,np.nan]).add(Series([2,3],index=[2,3]),fill_value=10)
Out[7]: 
0     NaN
1    11.0
2    12.0
3    13.0
dtype: float64

[facaiy]

I have modified the API, DataFrame._combine_const, is it OK?
if yes, for consistency, it seems that we need to modify _combine_const method of sparse and series and their caller .

[jreback]

you can just do

if fill_value is not None:
    self = self.fillna(fill_value)
data = self._data.eval(......)
....

we don't directly access .values like this as you can have multiple dtypes.

[facaiy]

Thanks, @jreback.

Since the _combine_const of all classes are designed without fill_value parameter, I worry whether modifying the API is a good idea. At least, Series and some member of sparse need do something to keep consistent. Could you give me some advice?

[facaiy]

How about use fillna here, instead of modifying the _combine_const method? @jreback

[facaiy]

If it is OK, I'd like to add a whatsnew and rebase.

[jreback]

specify this as np.arange(10, dtype='int64') or the comparison will fail on windows

[jreback]

pls test for the sparse cases as well

[facaiy]

I'm not familiar with sparse dataframe. And the sparse item and NaN are quite similar and confuse for me.

Traceback (most recent call last):
  File "/Users/facaiyan/Workshop/pandas/pandas/sparse/tests/test_sparse.py", line 966, in test_fill_value_missing_with_binary_operators
    tm.assert_sp_frame_equal(res, exp)
  File "/Users/facaiyan/Workshop/pandas/pandas/util/testing.py", line 1257, in assert_sp_frame_equal
    assert_sp_series_equal(series, right[col])
  File "/Users/facaiyan/Workshop/pandas/pandas/util/testing.py", line 1229, in assert_sp_series_equal
    assert_sp_array_equal(left.block.values, right.block.values)
  File "/Users/facaiyan/Workshop/pandas/pandas/util/testing.py", line 1205, in assert_sp_array_equal
    assert_almost_equal(left.sp_values, right.sp_values)
  File "/Users/facaiyan/Workshop/pandas/pandas/util/testing.py", line 124, in assert_almost_equal
    return _testing.assert_almost_equal(left, right, **kwargs)
  File "pandas/src/testing.pyx", line 58, in pandas._testing.assert_almost_equal (pandas/src/testing.c:3809)
  File "pandas/src/testing.pyx", line 120, in pandas._testing.assert_almost_equal (pandas/src/testing.c:2101)
  File "/Users/facaiyan/Workshop/pandas/pandas/util/testing.py", line 913, in raise_assert_detail
    raise AssertionError(msg)
AssertionError: numpy array are different

numpy array shapes are different
[left]:  (10,)
[right]: (9,)

[facaiy]

@jreback if I misunderstand, please tell me.

for sparse dataframe:

1. df.fillna(x): convert NaN entries of df to x.
2. In df.add(a, fill_value=x): fill_value, convert sparse entries of df to x.

[facaiy]

The work is done if all tests are passed.
Thanks, everyone.

[facaiy]

If it is ok,
might we close #12723 ?

[jreback]

issues are closed when the PRs are merged - I'll have a look soon

[jreback]

thanks for the fix!