From 4c92f86cc6ffd190cbab68d394ba4d820502cf1d Mon Sep 17 00:00:00 2001
From: Vyas Ramasubramani <vyasr@nvidia.com>
Date: Fri, 11 Feb 2022 09:45:26 -0800
Subject: [PATCH] Add tests of reflected ufuncs and fix behavior of logical
 reflected ufuncs (#10261)

This PR fixes cases like `cupy.array([1, 2, 3]) < cudf.Series([1, 2, 3])`. The code would previously attempt to find a reflected operator `Series.__rle__`, but no such operator exists because of the inherent symmetry in comparison operators. These changes fix the detection of such operators to just use operator `__ge__` in this case.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Michael Wang (https://github.com/isVoid)
  - GALI PREM SAGAR (https://github.com/galipremsagar)

URL: https://github.com/rapidsai/cudf/pull/10261
---
 python/cudf/cudf/core/series.py            | 24 +++++++++++++--
 python/cudf/cudf/tests/test_array_ufunc.py | 35 +++++++++++++++++-----
 2 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/python/cudf/cudf/core/series.py b/python/cudf/cudf/core/series.py
index 41bf75fb48f..ddaa358e9d9 100644
--- a/python/cudf/cudf/core/series.py
+++ b/python/cudf/cudf/core/series.py
@@ -998,9 +998,27 @@ def __array_ufunc__(self, ufunc, method, *inputs, **kwargs):
         # First look for methods of the class.
         fname = ufunc.__name__
         if fname in binary_operations:
-            not_reflect = self is inputs[0]
-            other = inputs[not_reflect]
-            op = f"__{'' if not_reflect else 'r'}{binary_operations[fname]}__"
+            reflect = self is not inputs[0]
+            other = inputs[0] if reflect else inputs[1]
+
+            # These operators need to be mapped to their inverses when
+            # performing a reflected operation because no reflected version of
+            # the operators themselves exist.
+            ops_without_reflection = {
+                "gt": "lt",
+                "ge": "le",
+                "lt": "gt",
+                "le": "ge",
+                # ne and eq are symmetric, so they are their own inverse op
+                "ne": "ne",
+                "eq": "eq",
+            }
+
+            op = binary_operations[fname]
+            if reflect and op in ops_without_reflection:
+                op = ops_without_reflection[op]
+                reflect = False
+            op = f"__{'r' if reflect else ''}{op}__"
 
             # pandas bitwise operations return bools if indexes are misaligned.
             # TODO: Generalize for other types of Frames
diff --git a/python/cudf/cudf/tests/test_array_ufunc.py b/python/cudf/cudf/tests/test_array_ufunc.py
index 2db0e8d29b8..a384ddecca6 100644
--- a/python/cudf/cudf/tests/test_array_ufunc.py
+++ b/python/cudf/cudf/tests/test_array_ufunc.py
@@ -1,3 +1,5 @@
+# Copyright (c) 2020-2022, NVIDIA CORPORATION.
+
 import operator
 from functools import reduce
 
@@ -98,17 +100,26 @@ def test_ufunc_series(ufunc, has_nulls, indexed):
         raise
 
 
-@pytest.mark.parametrize("ufunc", [np.add, np.greater, np.logical_and])
+@pytest.mark.parametrize(
+    "ufunc", [np.add, np.greater, np.greater_equal, np.logical_and]
+)
 @pytest.mark.parametrize("has_nulls", [True, False])
 @pytest.mark.parametrize("indexed", [True, False])
 @pytest.mark.parametrize("type_", ["cupy", "numpy", "list"])
-def test_binary_ufunc_series_array(ufunc, has_nulls, indexed, type_):
+@pytest.mark.parametrize("reflect", [True, False])
+def test_binary_ufunc_series_array(ufunc, has_nulls, indexed, type_, reflect):
     fname = ufunc.__name__
-    if fname == "greater" and has_nulls:
+    if fname in ("greater", "greater_equal") and has_nulls:
         pytest.xfail(
             "The way cudf casts nans in arrays to nulls during binops with "
             "cudf objects is currently incompatible with pandas."
         )
+    if reflect and has_nulls and type_ == "cupy":
+        pytest.skip(
+            "When cupy is the left operand there is no way for us to avoid "
+            "calling its binary operators, which cannot handle cudf objects "
+            "that contain nulls."
+        )
     N = 100
     # Avoid zeros in either array to skip division by 0 errors. Also limit the
     # scale to avoid issues with overflow, etc. We use ints because some
@@ -136,18 +147,28 @@ def test_binary_ufunc_series_array(ufunc, has_nulls, indexed, type_):
     if type_ == "list":
         arg1 = arg1.tolist()
 
-    got = ufunc(args[0], arg1)
-    expect = ufunc(args[0].to_pandas(), args[1].to_numpy())
+    if reflect:
+        got = ufunc(arg1, args[0])
+        expect = ufunc(args[1].to_numpy(), args[0].to_pandas())
+    else:
+        got = ufunc(args[0], arg1)
+        expect = ufunc(args[0].to_pandas(), args[1].to_numpy())
 
     if ufunc.nout > 1:
         for g, e in zip(got, expect):
             if has_nulls:
                 e[mask] = np.nan
-            assert_eq(g, e)
+            if type_ == "cupy" and reflect:
+                assert (cp.asnumpy(g) == e).all()
+            else:
+                assert_eq(g, e)
     else:
         if has_nulls:
             expect[mask] = np.nan
-        assert_eq(got, expect)
+        if type_ == "cupy" and reflect:
+            assert (cp.asnumpy(got) == expect).all()
+        else:
+            assert_eq(got, expect)
 
 
 @pytest.mark.parametrize(