Skip to content

Commit

Permalink
Deprecate binary operations with future unsupported types.
Browse files Browse the repository at this point in the history
  • Loading branch information
vyasr committed Mar 31, 2022
1 parent f321928 commit db84ddf
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 11 deletions.
13 changes: 10 additions & 3 deletions python/cudf/cudf/core/dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import sys
import warnings
from collections import defaultdict
from collections.abc import Iterable, Sequence
from collections.abc import Iterable, Mapping, Sequence
from typing import (
Any,
Dict,
Expand Down Expand Up @@ -1855,7 +1855,14 @@ def _make_operands_and_index_for_binop(
Optional[BaseIndex],
]:
# Check built-in types first for speed.
if isinstance(other, (list, dict, Sequence, MutableMapping)):
if isinstance(other, (list, dict, Sequence, Mapping)):
warnings.warn(
"Binary operations between host objects such as "
f"{type(other)} and cudf.DataFrame are deprecated and will be "
"removed in a future release. Please convert it to a cudf "
"object before performing the operation.",
FutureWarning,
)
if len(other) != self._num_columns:
raise ValueError(
"Other is the wrong length. Expected "
Expand Down Expand Up @@ -1898,7 +1905,7 @@ def _make_operands_and_index_for_binop(
# the fill value.
left_default = fill_value

if not isinstance(rhs, (dict, MutableMapping)):
if not isinstance(rhs, (dict, Mapping)):
return NotImplemented, None

operands = {
Expand Down
12 changes: 12 additions & 0 deletions python/cudf/cudf/core/single_column_frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from __future__ import annotations

import warnings
from typing import Any, Dict, Optional, Tuple, Type, TypeVar, Union

import cupy
Expand Down Expand Up @@ -333,6 +334,17 @@ def _make_operands_for_binop(
if isinstance(other, SingleColumnFrame):
other = other._column
elif not _is_scalar_or_zero_d_array(other):
if not hasattr(other, "__cuda_array_interface__"):
# TODO: When this deprecated behavior is removed, also change
# the above conditional to stop checking for pd.Series and
# pd.Index since we only need to support SingleColumnFrame.
warnings.warn(
f"Binary operations between host objects such as "
f"{type(other)} and {type(self)} are deprecated and will "
"be removed in a future release. Please convert it to a "
"cudf object before performing the operation.",
FutureWarning,
)
# Non-scalar right operands are valid iff they convert to columns.
try:
other = as_column(other)
Expand Down
31 changes: 23 additions & 8 deletions python/cudf/cudf/tests/test_dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import string
import textwrap
import warnings
from contextlib import contextmanager
from copy import copy

import cupy
Expand Down Expand Up @@ -2018,6 +2019,15 @@ def test_dataframe_min_count_ops(data, ops, skipna, min_count):
)


@contextmanager
def _hide_host_other_warning(other):
if isinstance(other, (dict, list)):
with pytest.warns(FutureWarning):
yield
else:
yield


@pytest.mark.parametrize(
"binop",
[
Expand Down Expand Up @@ -2068,17 +2078,22 @@ def test_binops_df(pdf, gdf, binop, other):
if isinstance(other, (pd.Series, pd.DataFrame)):
other = cudf.from_pandas(other)

assert_exceptions_equal(
lfunc=binop,
rfunc=binop,
lfunc_args_and_kwargs=([pdf, other], {}),
rfunc_args_and_kwargs=([gdf, other], {}),
compare_error_message=False,
)
# TODO: When we remove support for binary operations with lists and
# dicts, those cases should all be checked in a `pytest.raises` block
# that returns before we enter this try-except.
with _hide_host_other_warning(other):
assert_exceptions_equal(
lfunc=binop,
rfunc=binop,
lfunc_args_and_kwargs=([pdf, other], {}),
rfunc_args_and_kwargs=([gdf, other], {}),
compare_error_message=False,
)
else:
if isinstance(other, (pd.Series, pd.DataFrame)):
other = cudf.from_pandas(other)
g = binop(gdf, other)
with _hide_host_other_warning(other):
g = binop(gdf, other)
try:
assert_eq(d, g)
except AssertionError:
Expand Down

0 comments on commit db84ddf

Please sign in to comment.