Handle NaT types in isfinite utility #2716
Conversation
| return np.isnat(val) | ||
| else: | ||
| return val.view('i8') == nat_as_integer | ||
| elif pd and val is pd.NaT: |
There was a problem hiding this comment.
Has pd.NaT always been available, or only since a certain Pandas release? If the latter, is that Pandas release a minimum requirement for HoloViews?
There was a problem hiding this comment.
I see StackOverFlow issues about pd.NaT going back to 2014, so that doesn't seem a major worry.
There was a problem hiding this comment.
That said we should make a list of optional dependencies and determine minimum versions for them.
holoviews/core/util.py
Outdated
| elif numpy_version >= '1.13': | ||
| return ~np.isnat(val) | ||
| else: | ||
| return val.view('i8') != nat_as_integer |
There was a problem hiding this comment.
And this logic can't be folded into isnat above?
philippjfr
force-pushed
the
isfinite_nat
branch
2 times, most recently
from
972aa5d
to
0267a8d
Compare
|
Looks good although having to do those version checks for both numpy and pandas is rather annoying. Ready to merge? |
|
Yes, would be good to merge now since this is also needed to make #2719 work properly. |
|
Merging. |
As mentioned in #2715 we currently do not handle NaT types at all. Unfortunately this is a bit of a pain since it has only been consistently supported in NumPy >= 1.13. Additionally this PR improves the
isfiniteutility to work correctly on various arraylike types including pandas index and series types.