Repository: cupy/cupy
Pull request: Fix `cupy.clip` to match numpy
Fixes #6901 

Fixed the 'only `a_max` or `a_min` is given' issue by removing the default value of `None` for `a_min` and `a_max`.
Fixed the '`a_max` < `a_min`' issue by tweaking the cythonic implementation for the function.

Added test cases for the same.

Problem statement:
`cupy.clip` behave differently from NumPy
### Description

The behavior is different from NumPy when

* only `a_max` or `a_min` is given
* `a_max` < `a_min`

### To Reproduce

CuPy:
```py
>>> import cupy as cp
>>> cp.clip(cp.arange(8), a_min=3, a_max=6)
array([3, 3, 3, 3, 4, 5, 6, 6])
>>> cp.clip(cp.arange(8), a_min=3)
array([3, 3, 3, 3, 4, 5, 6, 7])
>>> cp.clip(cp.arange(8), a_max=3)
array([0, 1, 2, 3, 3, 3, 3, 3])
>>> cp.clip(cp.arange(8), a_min=6, a_max=3)
array([6, 6, 6, 6, 6, 6, 3, 3])
```

NumPy:
```py
>>> import numpy as np
>>> np.clip(np.arange(8), a_min=3, a_max=6)
array([3, 3, 3, 3, 4, 5, 6, 6])
>>> np.clip(np.arange(8), a_min=3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<__array_function__ internals>", line 179, in clip
TypeError: _clip_dispatcher() missing 1 required positional argument: 'a_max'
>>> np.clip(np.arange(8), a_max=3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<__array_function__ internals>", line 179, in clip
TypeError: _clip_dispatcher() missing 1 required positional argument: 'a_min'
>>> np.clip(np.arange(8), a_min=6, a_max=3)
array([3, 3, 3, 3, 3, 3, 3, 3])
```

### Installation

_No response_

### Environment

_No response_

### Additional Information

_No response_

Hints from issue discussion:
Will take this up.

Commit message: Coding style fix

Patch to review:
diff --git a/cupy/_core/_routines_math.pyx b/cupy/_core/_routines_math.pyx
index 5203f104167..ef8f5c288f7 100644
--- a/cupy/_core/_routines_math.pyx
+++ b/cupy/_core/_routines_math.pyx
@@ -1114,7 +1114,7 @@ _clip = create_ufunc(
     'cupy_clip',
     ('???->?', 'bbb->b', 'BBB->B', 'hhh->h', 'HHH->H', 'iii->i', 'III->I',
      'lll->l', 'LLL->L', 'qqq->q', 'QQQ->Q', 'eee->e', 'fff->f', 'ddd->d'),
-    'out0 = in0 < in1 ? in1 : (in0 > in2 ? in2 : in0)')
+    'out0 = in1 > in2 ? in2 : (in0 < in1 ? in1 : (in0 > in2 ? in2 : in0))')
 
 
 # Variables to expose to Python
diff --git a/cupy/_math/misc.py b/cupy/_math/misc.py
index 4f88e5faa97..11bb3a46a4c 100644
--- a/cupy/_math/misc.py
+++ b/cupy/_math/misc.py
@@ -144,7 +144,7 @@ def _dot_convolve(a1, a2, mode):
     return output
 
 
-def clip(a, a_min=None, a_max=None, out=None):
+def clip(a, a_min, a_max, out=None):
     """Clips the values of an array to a given interval.
 
     This is equivalent to ``maximum(minimum(a, a_max), a_min)``, while this
@@ -163,6 +163,10 @@ def clip(a, a_min=None, a_max=None, out=None):
 
     .. seealso:: :func:`numpy.clip`
 
+    Notes
+    -----
+    When `a_min` is greater than `a_max`, `clip` returns an
+    array in which all values are equal to `a_max`.
     """
     if fusion._is_fusing():
         return fusion._call_ufunc(_math.clip,
diff --git a/tests/cupy_tests/math_tests/test_misc.py b/tests/cupy_tests/math_tests/test_misc.py
index ce49f489355..846fdee866a 100644
--- a/tests/cupy_tests/math_tests/test_misc.py
+++ b/tests/cupy_tests/math_tests/test_misc.py
@@ -132,6 +132,19 @@ def test_external_clip2(self, xp, dtype):
         a = testing.shaped_arange((2, 3, 4), xp, dtype)
         return xp.clip(a, 3, 13)
 
+    @testing.for_all_dtypes(no_bool=True, no_complex=True)
+    @testing.numpy_cupy_array_equal()
+    def test_external_clip3(self, xp, dtype):
+        a = testing.shaped_arange((2, 3, 4), xp, dtype)
+        return xp.clip(a, 8, 1)
+
+    @testing.for_all_dtypes(no_bool=True, no_complex=True)
+    def test_external_clip4(self, dtype):
+        for xp in (numpy, cupy):
+            a = testing.shaped_arange((2, 3, 4), xp, dtype)
+            with pytest.raises(TypeError):
+                xp.clip(a, 3)
+
     @testing.for_all_dtypes(no_complex=True)
     @testing.numpy_cupy_array_equal()
     def test_clip2(self, xp, dtype):


Code review comments:
diff_hunk: @@ -132,6 +132,19 @@ def test_external_clip2(self, xp, dtype):
         a = testing.shaped_arange((2, 3, 4), xp, dtype)
         return xp.clip(a, 3, 13)
 
+    @testing.for_all_dtypes(no_bool=True, no_complex=True)
+    @testing.numpy_cupy_array_equal()
+    def test_external_clip3(self, xp, dtype):
+        a = testing.shaped_arange((2, 3, 4), xp, dtype)
+        return xp.clip(a, 8, 1)
path: tests/cupy_tests/math_tests/test_misc.py
text: @user1:
Could you fix the test to check also for the case of `x[i] < a_max`?
```suggestion
        return xp.clip(a, 8, 4)
```