ENH: add isinf, isnan, fmin, fmax loops for datetime64, timedelta64 #14841
Conversation
025cf09 to
0040d3e
Compare
|
In light of the failure of gh-14800, we should make sure this PR does not cause problems on pandas. |
|
Tested this against pandas 0.25.3. Some tests will need adjustment since now they do not raise, but it did not seem to cause new failures: |
numpy/testing/_private/utils.py
Outdated
| except (TypeError, ValueError, NotImplementedError): | ||
| except (TypeError, ValueError, NotImplementedError, DeprecationWarning): |
There was a problem hiding this comment.
This seems risky, and at least deserving of a source comment.
There was a problem hiding this comment.
Added an explanation and a version changed
There was a problem hiding this comment.
It's tempting to add an entirely separate except DeprecationWarning with this comment
There was a problem hiding this comment.
Thinking some more, we should not be catching warnings as exceptions at all - shouldn't we be suppressing them even if they're not exceptions (or not suppressing them at all)?
0040d3e to
259b6e3
Compare
numpygh-14841 failed to pass shippable tests, the CI could not install gfortran. We may need to add apt update before apt install.
|
Thanks! Note that import numpy as np
a = np.array(['1999-12-15', 'NaT'], dtype='M8[ns]')
np.nanmax(a)
np.nanmin(a)Both return |
|
the issue is that fmin, fmax are mapped to minimum, maximum when they should have unique loops to scan for NaT. I can add it to this PR. |
| facing code. Specifically, code that either disallowed the calls to `isinf`` or | ||
| ``isnan`` or checked that they raised an exception will require adaptation, and | ||
| code that mistakenly called ``fmax`` and ``fmin`` instead of ``maximum`` or | ||
| ``minimum`` respectively will requre adjustment. This also affects ``nanmax`` |
There was a problem hiding this comment.
I think nanmax raised before 1.18. (Or maybe I missunderstand this comment).
numpy/testing/_private/utils.py
Outdated
| # version 1.18 | ||
| # gisnan used to raise for datetime64/timedelta64 (dt64) since they had | ||
| # no isnan implementation. That call no longer raises. However, a | ||
| # DeprecationWarning is raised when comparing dt64 to 0. |
There was a problem hiding this comment.
this comment still applies - this will still emit noisy warnings if I don't have warnings set to raise - we should either:
- Suppress warnings, rather than catching them
- Locally promote warnings to raise
- Special case datetime types to avoid this path
- Call
cancastsomehow to avoid this path
There was a problem hiding this comment.
special casing seems like the easiest
| if (array_actual.dtype.char in 'Mm' or | ||
| array_desired.dtype.char in 'Mm'): |
There was a problem hiding this comment.
You only want M here, not m
There was a problem hiding this comment.
It doesn't really matter, the test just below this is only for things that will have a signbit on a 0 value, so timedelta64 might as well never go to the next clause
There was a problem hiding this comment.
Well at that point you may as well add integers here too - at any rate, the comment is saying "this won't work", not "this is a shortcut to save time".
There was a problem hiding this comment.
actually, np.timedelta64(7, 's') == 0 emits a DeprecationWarning, so we need both 'Mm', and the comment is wrong
There was a problem hiding this comment.
Seems weird that td[s] == td is deprecated yet td[s] + td is not - perhaps worth opening a new issue about
c5662d2 to
3483f80
Compare
|
Do you plan to get this into 1.18? I guess this could help external libraries so that they don't have to check for 3 cases
|
| ``np.datetime('NaT')`` should behave more like ``float('Nan')``. Add needed | ||
| infrastructure so ``np.isinf(a)`` and ``np.isnan(a)`` will run on | ||
| ``datetime64`` and ``timedelta64`` dtypes. Also added specific loops for | ||
| ``fmin`` and ``fmax`` that mask ``NaT``. This may require adjustment to user- |
There was a problem hiding this comment.
| ``fmin`` and ``fmax`` that mask ``NaT``. This may require adjustment to user- | |
| `fmin` and `fmax` that mask ``NaT``. This may require adjustment to user- |
may as well link these, right?
| ``datetime64`` and ``timedelta64`` dtypes. Also added specific loops for | ||
| `numpy.fmin` and `numpy.fmax` that mask ``NaT``. This may require adjustment to user- | ||
| facing code. Specifically, code that either disallowed the calls to | ||
| `numpy.isinf`` or `numpy.isnan`` or checked that they raised an exception will |
There was a problem hiding this comment.
| `numpy.isinf`` or `numpy.isnan`` or checked that they raised an exception will | |
| `numpy.isinf` or `numpy.isnan` or checked that they raised an exception will |
7fdcce7 to
2e25313
Compare
Co-Authored-By: Warren Weckesser <warren.weckesser@gmail.com>
Co-Authored-By: Warren Weckesser <warren.weckesser@gmail.com>
Fixes gh-14831.
Changes behavior. Previously
isnan,isinfwere unavailable fordatetime64dtypes. Nowassert_equalbehaves slightly differently. Previouslyassert_equal(a, b)would raise ifaandbcontainedNaTin the same position. NowNaTis equivalent tofloat('nan')and the call passes. I think this is consistent with the function docstring: