Repository: numpy/numpy
Pull request: API: Disallow strings in logical ufuncs
This restores pre 1.22 NumPy behaviour for strings passed into logical
ufuncs.  Logical ufuncs should be able to cast inputs to booleans
without modifying their results.
Thus they do this now (in NumPy 1.22+).  The problem is that string to
bool casts are very strange in NumPy currently.

This should be reverted/removed once string to bool casts are well
defined.  But until then, it seems more reasonable to just reject
strings.

Closes gh-20898


Problem statement:
BUG: Vague error message from ufunc `np.any`
### Describe the issue:

Noticed with NumPy 1.22 that the error message from `np.any` is a bit vague. However NumPy 1.21 has a clearer error message. Interestingly the error type raised changed from `TypeError` to `ValueError`.

### Reproduce the code example:

```python
In [1]: import numpy as np

In [2]: a = np.array(["baz", "baz", "baz", "bar", "foo"], dtype="<U3")

In [3]: a.any()
```


### Error message:

The NumPy 1.22 error message is:

```python
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Input In [3], in <module>
----> 1 a.any()

File ~/miniconda/envs/np122/lib/python3.10/site-packages/numpy/core/_methods.py:57, in _any(a, axis, dtype, out, keepdims, where)
     54 def _any(a, axis=None, dtype=None, out=None, keepdims=False, *, where=True):
     55     # Parsing keyword arguments is currently fairly slow, so avoid it for now
     56     if where is True:
---> 57         return umr_any(a, axis, dtype, out, keepdims)
     58     return umr_any(a, axis, dtype, out, keepdims, where=where)

ValueError: invalid literal for int() with base 10: 'baz'
```

Compare this to the previous error message from NumPy 1.21, which was:

```python
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Input In [3], in <module>
----> 1 a.any()

File ~/miniconda/envs/np121/lib/python3.10/site-packages/numpy/core/_methods.py:57, in _any(a, axis, dtype, out, keepdims, where)
     54 def _any(a, axis=None, dtype=None, out=None, keepdims=False, *, where=True):
     55     # Parsing keyword arguments is currently fairly slow, so avoid it for now
     56     if where is True:
---> 57         return umr_any(a, axis, dtype, out, keepdims)
     58     return umr_any(a, axis, dtype, out, keepdims, where=where)

TypeError: cannot perform reduce with flexible type
```

### NumPy/Python version information:

```
1.22.1 3.10.2 | packaged by conda-forge | (main, Jan 14 2022, 08:02:37) [Clang 11.1.0 ]
```

Hints from issue discussion:
Hmmm, it now behaves like `arr.astype(bool)` instead of failing immediately.  Which I think is better in principle, but the truthiness of NumPy strings is a giant mess still :(.
At least from a user perspective, it wasn't clear to me what was going wrong whereas the original error message this was clear
We discussed this a bit (in the triage meeting), and settled on just rejecting strings explicitly for now, because this is a bit weird.

Commit message: API: Disallow strings in logical ufuncs

This restores pre 1.22 NumPy behaviour for strings passed into logical
ufuncs.  Logical ufuncs should be able to cast inputs to booleans
without modifying their results.
Thus they do this now (in NumPy 1.22+).  The problem is that string to
bool casts are very strange in NumPy currently.

This should be reverted/removed once string to bool casts are well
defined.  But until then, it seems more reasonable to just reject
strings.

Closes gh-20898

Patch to review:
diff --git a/numpy/core/src/umath/dispatching.c b/numpy/core/src/umath/dispatching.c
index 6f541340e2bc..83973b57daba 100644
--- a/numpy/core/src/umath/dispatching.c
+++ b/numpy/core/src/umath/dispatching.c
@@ -1018,6 +1018,11 @@ logical_ufunc_promoter(PyUFuncObject *NPY_UNUSED(ufunc),
         /* bail out, this is _only_ to give future/deprecation warning! */
         return -1;
     }
+    if ((op_dtypes[0] != NULL && PyTypeNum_ISSTRING(op_dtypes[0]->type_num))
+            || PyTypeNum_ISSTRING(op_dtypes[1]->type_num)) {
+        /* bail out on strings: currently caststing them to bool is too weird */
+        return -1;
+    }
 
     for (int i = 0; i < 3; i++) {
         PyArray_DTypeMeta *item;
diff --git a/numpy/core/tests/test_ufunc.py b/numpy/core/tests/test_ufunc.py
index 41f9778b6431..f13c2667d762 100644
--- a/numpy/core/tests/test_ufunc.py
+++ b/numpy/core/tests/test_ufunc.py
@@ -2134,7 +2134,7 @@ def test_logical_ufuncs_mixed_object_signatures(self, ufunc, signature):
             [np.logical_and, np.logical_or, np.logical_xor])
     def test_logical_ufuncs_support_anything(self, ufunc):
         # The logical ufuncs support even input that can't be promoted:
-        a = np.array('1')
+        a = np.array(b'1', dtype="V3")
         c = np.array([1., 2.])
         assert_array_equal(ufunc(a, c), ufunc([True, True], True))
         assert ufunc.reduce(a) == True
@@ -2150,6 +2150,23 @@ def test_logical_ufuncs_support_anything(self, ufunc):
         out = np.zeros((), dtype=a.dtype)
         assert ufunc.reduce(a, out=out) == 1
 
+    @pytest.mark.parametrize("ufunc",
+            [np.logical_and, np.logical_or, np.logical_xor])
+    def test_logical_ufuncs_reject_string(self, ufunc):
+        """
+        Logical ufuncs are normally well defined by working with the boolean
+        equivalent, i.e. casting all inputs to bools should work.
+
+        However, casting strings to bools is *currently* weird, because it
+        actually uses `bool(int(str))`.  Thus we explicitly reject strings.
+        This test should succeed (and can probably just be removed) as soon as
+        string to bool casts are well defined in NumPy.
+        """
+        with pytest.raises(TypeError, match="contain a loop with signature"):
+            ufunc(["1"], ["3"])
+        with pytest.raises(TypeError, match="contain a loop with signature"):
+            ufunc.reduce(["1", "2", "0"])
+
     @pytest.mark.parametrize("ufunc",
              [np.logical_and, np.logical_or, np.logical_xor])
     def test_logical_ufuncs_out_cast_check(self, ufunc):


Code review comments:
diff_hunk: @@ -1018,6 +1018,11 @@ logical_ufunc_promoter(PyUFuncObject *NPY_UNUSED(ufunc),
         /* bail out, this is _only_ to give future/deprecation warning! */
         return -1;
     }
+    if ((op_dtypes[0] != NULL && PyTypeNum_ISSTRING(op_dtypes[0]->type_num))
+            || PyTypeNum_ISSTRING(op_dtypes[1]->type_num)) {
+        /* bail out on strings: currently caststing them to bool is too weird */
path: numpy/core/src/umath/dispatching.c
text: @user1:
```suggestion
        /* bail out on strings: currently casting them to bool is too weird */
```