Repository: numpy/numpy
Pull request: ENH: `isfinite` support for `datetime64` and `timedelta64`
This fixes #5610.

Works by adding a `PyUFunc_IsFiniteTypeResolver` for `isfinite` ufunc. It does what `PyUFunc_IsNaTTypeResolver` does for `datetime64` and `timedelta64` objects, and calls  `PyUFunc_DefaultTypeResolver` for all other dtypes.



Problem statement:
Add `isfinite` support for `datetime64` and `timedelta64`
`numpy.isfinite` should support `datetime64` and `timedelta64` objects.  It should return `True` for any ordinary value, and `False` for `datetime64('NaT')` and `timedelta64('NaT')`.

As a side-effect, this will make `isclose` and `allclose` usable for `datetime64` and `timedelta64` objects.

```
In [97]: numpy.isfinite(numpy.datetime64(10**9, 's'))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-97-8555b18406e5> in <module>()
----> 1 numpy.isfinite(numpy.datetime64(10**9, 's'))

TypeError: ufunc 'isfinite' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe''
```

Hints from issue discussion:
I made some progress on this today: https://github.com/numpy/numpy/commit/cc99cdd53de418cb7a00d580676c87246108e096 / 
https://github.com/shoyer/numpy/tree/datetime64-isfinite

It works for unit-less datetime64/timedelta64, but fails for dates or times with units attached:

```
In [1]: import numpy as np

In [2]: np.isfinite(np.timedelta64(10))
Out[2]: True

In [3]: np.isfinite(np.timedelta64('NaT'))
Out[3]: False

In [4]: np.isfinite(np.timedelta64('NaT', 'us'))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-3998f88199cc> in <module>()
----> 1 np.isfinite(np.timedelta64('NaT', 'us'))

TypeError: ufunc 'isfinite' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe''
```

It appears that this needs something with the ufunc dtype resolution to handle datetimes with units, but I'm not quite sure what that is. Somehow `PyUFunc_SimpleBinaryComparisonTypeResolver` and `PyUFunc_SimpleUnaryOperationTypeResolver` can handle datetime64 with units but the default `PyUFunc_DefaultTypeResolver` cannot.

@shoyer I was wondering whether you got somewhere with this? I fear that if we change the NaT comparison handling, we probably cannot avoid implementing `isnan` and `isfinite` for it.
Otherwise even our own tests should not like them (though of course we could work around that).

EDIT: See also gh-5222

@seberg nope, I got stuck on ufunc type resolution (in the linked branch). I do agree this would be useful, though I usually find checking for `x != x` is a good way to identify NaN/NaT even without a dedicated function.

I just hit this bug. I hope it can be fixed.

I took a look at @shoyer's branch at https://github.com/numpy/numpy/commit/cc99cdd53de418cb7a00d580676c87246108e096.  The issue seems to be that the default type resolver ends up invoking [`linear_search_type_resolver`](https://github.com/numpy/numpy/blob/master/numpy/core/src/umath/ufunc_type_resolution.c#L1867), which essentially executes something like:

```
for type in ufunc.registered_types:
    if ufunc_loop_matches(input_data, type, ...):
       <use type>
```

(the actual code here is https://github.com/numpy/numpy/blob/master/numpy/core/src/umath/ufunc_type_resolution.c#L1867).

For at least the scalar case, `ufunc_loop_matches` essentially boils down to a call to `PyArray_CanCastTypeTo`: https://github.com/numpy/numpy/blob/master/numpy/core/src/umath/ufunc_type_resolution.c#L1529-L1535 with a casting rule of `'safe'`.

The issue ends up being that the input dtype is `datetime64[<unit>]` or `timedelta64[<unit>]`, but the type registered for the ufunc is just vanilla `datetime64` or `timedelta64`.  Since casting from a datetime with a unit to one without a unit isn't deemed, safe, we decide that we don't have a ufunc registered for the input data.  (This is why the dispatch works correctly on unit-less datetimes/timedeltas.).

I think the easiest fix here is probably to just add a new ufunc type resolver for `isfinite`.  It doesn't seem worthwhile to try to shoehorn datetime/timedelta-handling logic into the default resolver, since almost all ufuncs that work with datetimes/timedeltas seem to need their own custom behavior to deal with unit conversion (there are already custom resolvers used for `add`, `subtract` and `multiply` for example.

@ssanderson would it also work to register the datetime types as types to `np.isfinite` and `np.isnan` (assuming we are OK using those names)? Honestly, have not tried to understand the issues yet really.

If you ahve some time, we all always appreciate pull requests!

@seberg I don't think it's possible to register ufuncs for the specific unit.  Looking at the signature for `PyUFunc_FromFuncAndData`, it looks like you can only provide type numbers, but the units are metadata attached to the type number.

I made some progress on a custom resolver last night; I'll see if I can tie off that work tonight.

It's possibly worth figuring out what path in ufunc_type_resolution `isnan`/`isfinite` normally take (for float data) and why that doesn't work for datetime64.

@shoyer I think my long post above outlines the path they take.  

The root issue is that part of (at least the generic case) of ufunc resolution is checking whether the input dtype can be safely cast to a dtype for which there's a registered ufunc for the function being dispatched.  ufuncs that work on datetime/timedelta are always just registered with a generic dtype, and it's not considered safe to cast from `datetime64[<unit>]` to generic `datetime64` (which seems correct in general?) which means that, as far as I can tell, every ufunc that works on datetime/timedelta needs a custom resolver to work correctly.  

The only exception seems to be ufuncs dispatched with `PyUFunc_SimpleBinaryComparisonTypeResolver`, which directly invokes `PyArray_ResultType` to figure out how to coerce the input, and then always yields `NPY_BOOL` for the output.  Something similar, e.g. a `PyUFunc_SimpleUnaryPredicateTypeResolver` for functions that always yield NPY_BOOL would probably work for `isfinite`, and `isnan` if we wanted to make that work for datetime (I'd personally be :-1: on that: I'd rather have a generic `isnull` than have `isnan` work on non-floating values).

Commit message: TST: testing isfinite on datetime and timedelta objects.
Added test: test_datetime.TestDateTime.test_isfinite.
It just checks the reverse of test_isnat.

Patch to review:
diff --git a/numpy/core/code_generators/generate_umath.py b/numpy/core/code_generators/generate_umath.py
index de0bb81fe9bb..d5357ac7de4b 100644
--- a/numpy/core/code_generators/generate_umath.py
+++ b/numpy/core/code_generators/generate_umath.py
@@ -844,8 +844,8 @@ def english_upper(s):
 'isfinite':
     Ufunc(1, 1, None,
           docstrings.get('numpy.core.umath.isfinite'),
-          None,
-          TD(nodatetime_or_obj, out='?'),
+          'PyUFunc_IsFiniteTypeResolver',
+          TD(nodatetime_or_obj + times, out='?'),
           ),
 'signbit':
     Ufunc(1, 1, None,
diff --git a/numpy/core/src/umath/loops.c.src b/numpy/core/src/umath/loops.c.src
index 1e4ab350bec9..1e65acd3b4b3 100644
--- a/numpy/core/src/umath/loops.c.src
+++ b/numpy/core/src/umath/loops.c.src
@@ -1166,6 +1166,15 @@ NPY_NO_EXPORT void
     }
 }
 
+NPY_NO_EXPORT void
+@TYPE@_isfinite(char **args, npy_intp *dimensions, npy_intp *steps, void *NPY_UNUSED(func))
+{
+    UNARY_LOOP {
+        const @type@ in1 = *(@type@ *)ip1;
+        *((npy_bool *)op1) = (in1 != NPY_DATETIME_NAT);
+    }
+}
+
 NPY_NO_EXPORT void
 @TYPE@__ones_like(char **args, npy_intp *dimensions, npy_intp *steps, void *NPY_UNUSED(data))
 {
diff --git a/numpy/core/src/umath/loops.h.src b/numpy/core/src/umath/loops.h.src
index f48319056c26..9dc1b70165f8 100644
--- a/numpy/core/src/umath/loops.h.src
+++ b/numpy/core/src/umath/loops.h.src
@@ -427,6 +427,9 @@ TIMEDELTA_sign(char **args, npy_intp *dimensions, npy_intp *steps, void *NPY_UNU
 NPY_NO_EXPORT void
 @TYPE@_isnat(char **args, npy_intp *dimensions, npy_intp *steps, void *NPY_UNUSED(func));
 
+NPY_NO_EXPORT void
+@TYPE@_isfinite(char **args, npy_intp *dimensions, npy_intp *steps, void *NPY_UNUSED(func));
+
 NPY_NO_EXPORT void
 @TYPE@__ones_like(char **args, npy_intp *dimensions, npy_intp *steps, void *NPY_UNUSED(data));
 
diff --git a/numpy/core/src/umath/ufunc_type_resolution.c b/numpy/core/src/umath/ufunc_type_resolution.c
index c07934caaf8f..b2400a2a16ab 100644
--- a/numpy/core/src/umath/ufunc_type_resolution.c
+++ b/numpy/core/src/umath/ufunc_type_resolution.c
@@ -611,6 +611,26 @@ PyUFunc_IsNaTTypeResolver(PyUFuncObject *ufunc,
     return 0;
 }
 
+
+NPY_NO_EXPORT int
+PyUFunc_IsFiniteTypeResolver(PyUFuncObject *ufunc,
+                          NPY_CASTING casting,
+                          PyArrayObject **operands,
+                          PyObject *type_tup,
+                          PyArray_Descr **out_dtypes)
+{
+    if (!PyTypeNum_ISDATETIME(PyArray_DESCR(operands[0])->type_num)) {
+        return PyUFunc_DefaultTypeResolver(ufunc, casting, operands,
+                                    type_tup, out_dtypes);
+    }
+
+    out_dtypes[0] = ensure_dtype_nbo(PyArray_DESCR(operands[0]));
+    out_dtypes[1] = PyArray_DescrFromType(NPY_BOOL);
+
+    return 0;
+}
+
+
 /*
  * Creates a new NPY_TIMEDELTA dtype, copying the datetime metadata
  * from the given dtype.
diff --git a/numpy/core/src/umath/ufunc_type_resolution.h b/numpy/core/src/umath/ufunc_type_resolution.h
index 7256fcf61678..a4e670a8e56b 100644
--- a/numpy/core/src/umath/ufunc_type_resolution.h
+++ b/numpy/core/src/umath/ufunc_type_resolution.h
@@ -43,6 +43,13 @@ PyUFunc_IsNaTTypeResolver(PyUFuncObject *ufunc,
                           PyObject *type_tup,
                           PyArray_Descr **out_dtypes);
 
+NPY_NO_EXPORT int
+PyUFunc_IsFiniteTypeResolver(PyUFuncObject *ufunc,
+                             NPY_CASTING casting,
+                             PyArrayObject **operands,
+                             PyObject *type_tup,
+                             PyArray_Descr **out_dtypes);
+
 NPY_NO_EXPORT int
 PyUFunc_AdditionTypeResolver(PyUFuncObject *ufunc,
                              NPY_CASTING casting,
diff --git a/numpy/core/tests/test_datetime.py b/numpy/core/tests/test_datetime.py
index 91cd2c115229..f48bfdf9b64d 100644
--- a/numpy/core/tests/test_datetime.py
+++ b/numpy/core/tests/test_datetime.py
@@ -2208,6 +2208,27 @@ def test_isnat_error(self):
                 continue
             assert_raises(TypeError, np.isnat, np.zeros(10, t))
 
+    def test_isfinite(self):
+        assert_(not np.isfinite(np.datetime64('NaT', 'ms')))
+        assert_(not np.isfinite(np.datetime64('NaT', 'ns')))
+        assert_(np.isfinite(np.datetime64('2038-01-19T03:14:07')))
+
+        assert_(not np.isfinite(np.timedelta64('NaT', "ms")))
+        assert_(np.isfinite(np.timedelta64(34, "ms")))
+
+        res = np.array([True, True,  False])
+        for unit in ['Y', 'M', 'W', 'D',
+                     'h', 'm', 's', 'ms', 'us',
+                     'ns', 'ps', 'fs', 'as']:
+            arr = np.array([123, -321, "NaT"], dtype='<datetime64[%s]' % unit)
+            assert_equal(np.isfinite(arr), res)
+            arr = np.array([123, -321, "NaT"], dtype='>datetime64[%s]' % unit)
+            assert_equal(np.isfinite(arr), res)
+            arr = np.array([123, -321, "NaT"], dtype='<timedelta64[%s]' % unit)
+            assert_equal(np.isfinite(arr), res)
+            arr = np.array([123, -321, "NaT"], dtype='>timedelta64[%s]' % unit)
+            assert_equal(np.isfinite(arr), res)
+
     def test_corecursive_input(self):
         # construct a co-recursive list
         a, b = [], []


Code review comments:
diff_hunk: @@ -844,8 +844,8 @@ def english_upper(s):
 'isfinite':
     Ufunc(1, 1, None,
           docstrings.get('numpy.core.umath.isfinite'),
-          None,
-          TD(nodatetime_or_obj, out='?'),
+          'PyUFunc_IsFiniteTypeResolver',
+          TD(nodatetime_or_obj + times, out='?'),
path: numpy/core/code_generators/generate_umath.py
text: @user1:
Shorter as just `noobj` (tested to be equivalent on my machine)