Repository: pandas-dev/pandas
Pull request: REGR: fix op(frame, frame2) with reindex
- [x] closes #31623
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

cc @TomAugspurger this is pretty ugly, and I'm not sure how well it will behave if either frame has MultiIndex colums.

On the plus side, it could improve perf in the many-columns-but-small-intersection case.

The ugliness might be improved by moving this check to before the _align_method_FRAME call

Problem statement:
REGR: AssertionError when subtracting Timestamp-valued DataFrames with non-indentical column index
```python
import pandas as pd

df = pd.DataFrame(
    {
        "foo": [pd.Timestamp("2019"), pd.Timestamp("2020")],
        "bar": [pd.Timestamp("2018"), pd.Timestamp("2021")],
    }
)

df2 = df[["foo"]]

print(df - df2)
```
#### Problem description

The above snippet raises the following exception:

```
Traceback (most recent call last):
  File ".venv/lib/python3.6/site-packages/pandas/core/ops/array_ops.py", line 149, in na_arithmetic_op
    result = expressions.evaluate(op, str_rep, left, right)
  File ".v
env/lib/python3.6/site-packages/pandas/core/computation/expressions.py", line 208, in evaluate
    return _evaluate(op, op_str, a, b)
  File ".venv/lib/python3.6/site-packages/pandas/core/computation/expressions.py", line 70, in _evaluate_standard
    return op(a, b)
  File ".venv/lib/python3.6/site-packages/pandas/core/ops/common.py", line 64, in new_method
    return method(self, other)
  File ".venv/lib/python3.6/site-packages/pandas/core/ops/__init__.py", line 500, in wrapper
    result = arithmetic_op(lvalues, rvalues, op, str_rep)
  File ".venv/lib/python3.6/site-packages/pandas/core/ops/array_ops.py", line 192, in arithmetic_op
    res_values = dispatch_to_extension_op(op, lvalues, rvalues)
  File ".venv/lib/python3.6/site-packages/pandas/core/ops/dispatch.py", line 125, in dispatch_to_extension_op
    res_values = op(left, right)
  File ".venv/lib/python3.6/site-packages/pandas/core/arrays/datetimelike.py", line 1390, in __rsub__
    f"cannot subtract {type(self).__name__} from {type(other).__name__}"
TypeError: cannot subtract DatetimeArray from ndarray

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "pandas_bug.py", line 36, in <module>
    print(df2 - df)
  File ".venv/lib/python3.6/site-packages/pandas/core/ops/__init__.py", line 703, in f
    new_data = left._combine_frame(right, pass_op, fill_value)
  File ".venv/lib/python3.6/site-packages/pandas/core/frame.py", line 5297, in _combine_frame
    new_data = ops.dispatch_to_series(self, other, _arith_op)
  File ".venv/lib/python3.6/site-packages/pandas/core/ops/__init__.py", line 416, in dispatch_to_series
    new_data = expressions.evaluate(column_op, str_rep, left, right)
  File ".venv/lib/python3.6/site-packages/pandas/core/computation/expressions.py", line 208, in evaluate
    return _evaluate(op, op_str, a, b)
  File ".venv/lib/python3.6/site-packages/pandas/core/computation/expressions.py", line 70, in _evaluate_standard
    return op(a, b)
  File ".venv/lib/python3.6/site-packages/pandas/core/ops/__init__.py", line 385, in column_op
    return {i: func(a.iloc[:, i], b.iloc[:, i]) for i in range(len(a.columns))}
  File ".venv/lib/python3.6/site-packages/pandas/core/ops/__init__.py", line 385, in <dictcomp>
    return {i: func(a.iloc[:, i], b.iloc[:, i]) for i in range(len(a.columns))}
  File ".venv/lib/python3.6/site-packages/pandas/core/ops/array_ops.py", line 121, in na_op
    return na_arithmetic_op(x, y, op, str_rep)
  File ".venv/lib/python3.6/site-packages/pandas/core/ops/array_ops.py", line 151, in na_arithmetic_op
    result = masked_arith_op(left, right, op)
  File ".venv/lib/python3.6/site-packages/pandas/core/ops/array_ops.py", line 75, in masked_arith_op
    assert isinstance(x, np.ndarray), type(x)
```
This is a 1.0.0 regression; in 0.25.3, the operation succeeds and the unmatched `bar` column is filled with `NaN` in the output.

The same error occurs with:
* Any combination of incompatible columns (strict subset, strict superset, overlapping, disjoint)
* Calling the `subtract` method instead of using the subtraction operator
* Timezone-aware `Timestamp`s as well as timezone-naive

It does *not* seem to occur with:
* Mismatches on the row index; transposing the dataframes in the above example prevents the errors occuring.
* `pd.Series` objects with mismatched indexes (e.g. calling the above on the first row of each dataframe works fine)
* Other dtypes; `bool`, `float`, and `int` seem to work fine. Similarly, if the dataframes are explicitly cast to dtype `object`, the operation succeeds.

#### Expected Output

```
   bar    foo
0  NaN 0 days
1  NaN 0 days
```

#### Output of ``pd.show_versions()``

<details>
```
INSTALLED VERSIONS
------------------
commit           : None
python           : 3.6.8.final.0
python-bits      : 64
OS               : Linux
OS-release       : 4.15.0-74-generic
machine          : x86_64
processor        : x86_64
byteorder        : little
LC_ALL           : None
LANG             : en_GB.UTF-8
LOCALE           : en_GB.UTF-8

pandas           : 1.0.0
numpy            : 1.18.1
pytz             : 2019.3
dateutil         : 2.8.1
pip              : 19.3.1
setuptools       : 41.6.0
Cython           : None
pytest           : 5.3.5
hypothesis       : None
sphinx           : None
blosc            : None
feather          : None
xlsxwriter       : None
lxml.etree       : None
html5lib         : None
pymysql          : None
psycopg2         : None
jinja2           : None
IPython          : None
pandas_datareader: None
bs4              : None
bottleneck       : None
fastparquet      : None
gcsfs            : None
lxml.etree       : None
matplotlib       : None
numexpr          : None
odfpy            : None
openpyxl         : None
pandas_gbq       : None
pyarrow          : None
pytables         : None
pytest           : 5.3.5
pyxlsb           : None
s3fs             : None
scipy            : 1.4.1
sqlalchemy       : None
tables           : None
tabulate         : None
xarray           : None
xlrd             : None
xlwt             : None
xlsxwriter       : None
numba            : None
```

</details>

Hints from issue discussion:
Thanks for the report. The NaNs are introduced in https://github.com/pandas-dev/pandas/blob/a2721fd602e43128314d4efd056dae56a89197bf/pandas/core/ops/__init__.py#L725, which calls DataFrame.align.

I wonder, should this be changed?

```python
In [6]: df.align(df2)[1]
Out[6]:
   bar        foo
0  NaN 2019-01-01
1  NaN 2020-01-01
```

to have `bar` be datetime64[ns] dtype, to match the left?
cc @jbrockmendel.
ill look at this today
So this is pretty ugly, but one option that tentatively works is to patch ops._arith_method_FRAME so that we only operate on shared columns, then reindex the result.
might actually improve perf for cases where we have very few shared columns
That seems reasonable. The alternative is to ensure that the correct `fill_value` is used in align, which seems difficult since we'd potentially have different fill values for different columns / dtypes.

Is that likely to cause issues with methods like `DataFrame.add`? I forget whether the fill_value from add is done before or after the op.
> The alternative is to ensure that the correct fill_value is used in align, which seems difficult since we'd potentially have different fill values for different columns / dtypes.

yah, it would also depend on op, which would become a nightmare.

I'll put up a proof of concept in a bit

Commit message: POC: fix op(frame, frame2) with reindex

Patch to review:
diff --git a/pandas/core/ops/__init__.py b/pandas/core/ops/__init__.py
index 0312c11a6d590..1b988a54a65de 100644
--- a/pandas/core/ops/__init__.py
+++ b/pandas/core/ops/__init__.py
@@ -722,6 +722,7 @@ def _arith_method_FRAME(cls, op, special):
     @Appender(doc)
     def f(self, other, axis=default_axis, level=None, fill_value=None):
 
+        orig_self, orig_other = self, other
         self, other = _align_method_FRAME(self, other, axis, flex=True, level=level)
 
         if isinstance(other, ABCDataFrame):
@@ -729,6 +730,20 @@ def f(self, other, axis=default_axis, level=None, fill_value=None):
             pass_op = op if should_series_dispatch(self, other, op) else na_op
             pass_op = pass_op if not is_logical else op
 
+            if isinstance(orig_other, ABCDataFrame):
+                if fill_value is None and level is None and axis is default_axis:
+                    # TODO: any other cases we should handle here?
+                    cols = orig_self.columns.intersection(orig_other.columns)
+                    if not (
+                        cols.equals(orig_self.columns)
+                        and cols.equals(orig_other.columns)
+                    ):
+                        # GH#31623, only operate on shared columns
+                        new_left = orig_self[cols]
+                        new_right = orig_other[cols]
+                        result = op(new_left, new_right)
+                        return result.reindex(self.columns, axis=1)
+
             new_data = self._combine_frame(other, pass_op, fill_value)
             return self._construct_result(new_data)
 
diff --git a/pandas/tests/frame/test_arithmetic.py b/pandas/tests/frame/test_arithmetic.py
index c6eacf2bbcd84..44ad55517dcea 100644
--- a/pandas/tests/frame/test_arithmetic.py
+++ b/pandas/tests/frame/test_arithmetic.py
@@ -711,6 +711,25 @@ def test_operations_with_interval_categories_index(self, all_arithmetic_operator
         expected = pd.DataFrame([[getattr(n, op)(num) for n in data]], columns=ind)
         tm.assert_frame_equal(result, expected)
 
+    def test_frame_with_frame_reindex(self):
+        # GH#31623
+        df = pd.DataFrame(
+            {
+                "foo": [pd.Timestamp("2019"), pd.Timestamp("2020")],
+                "bar": [pd.Timestamp("2018"), pd.Timestamp("2021")],
+            },
+            columns=["foo", "bar"],
+        )
+        df2 = df[["foo"]]
+
+        result = df - df2
+
+        expected = pd.DataFrame(
+            {"foo": [pd.Timedelta(0), pd.Timedelta(0)], "bar": [np.nan, np.nan]},
+            columns=["bar", "foo"],
+        )
+        tm.assert_frame_equal(result, expected)
+
 
 def test_frame_with_zero_len_series_corner_cases():
     # GH#28600


Code review comments:
diff_hunk: @@ -722,13 +722,28 @@ def _arith_method_FRAME(cls, op, special):
     @Appender(doc)
     def f(self, other, axis=default_axis, level=None, fill_value=None):
 
+        orig_self, orig_other = self, other
         self, other = _align_method_FRAME(self, other, axis, flex=True, level=level)
 
         if isinstance(other, ABCDataFrame):
             # Another DataFrame
             pass_op = op if should_series_dispatch(self, other, op) else na_op
             pass_op = pass_op if not is_logical else op
 
+            if isinstance(orig_other, ABCDataFrame):
+                if fill_value is None and level is None and axis is default_axis:
+                    # TODO: any other cases we should handle here?
+                    cols = orig_self.columns.intersection(orig_other.columns)
+                    if not (
+                        cols.equals(orig_self.columns)
+                        and cols.equals(orig_other.columns)
+                    ):
+                        # GH#31623, only operate on shared columns
+                        new_left = orig_self[cols]
+                        new_right = orig_other[cols]
+                        result = op(new_left, new_right)
+                        return result.reindex(self.columns, axis=1)
path: pandas/core/ops/__init__.py
text: @user1:
This should be the union of the columns right?

@author:
i think so, but id have to double-check the exact outer-join semantics

diff_hunk: @@ -722,13 +722,28 @@ def _arith_method_FRAME(cls, op, special):
     @Appender(doc)
     def f(self, other, axis=default_axis, level=None, fill_value=None):
 
+        orig_self, orig_other = self, other
         self, other = _align_method_FRAME(self, other, axis, flex=True, level=level)
 
         if isinstance(other, ABCDataFrame):
             # Another DataFrame
             pass_op = op if should_series_dispatch(self, other, op) else na_op
             pass_op = pass_op if not is_logical else op
 
+            if isinstance(orig_other, ABCDataFrame):
path: pandas/core/ops/__init__.py
text: @user1:
shouldn't this be handled in .align? why is this needed at all?