Skip to content

ENH: isfinite support for datetime64 and timedelta64 #13218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 19, 2019

Conversation

debsankha
Copy link

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.

seberg and others added 3 commits March 30, 2019 11:49
I am not quite sure whether this should be done somewhat more elegant
especially with the typeresolver.
Added test: test_datetime.TestDateTime.test_isfinite.
It just checks the reverse of test_isnat.
@seberg
Copy link
Member

seberg commented Mar 30, 2019

I think quickly checking how to fix the isfinite version, we forgot that for histogram with automatic bins np.linspace would have to be fixed as well (not sure how straight forward that is). Although, I guess this probably still doesn't hurt, just does not quite fix the linked issue.

@debsankha
Copy link
Author

debsankha commented Mar 30, 2019

@seberg : That's true. I looked around a bit, and found the following:

As per my novice understanding, both linspace and histogram needs special logic to deal with datetime64 and timedelta64. And maybe some ufunc level changes might help, e.g. multiply between timedelta64 and float.

@debsankha
Copy link
Author

I have made an attempt to get linspace to work with datetime64 and timedelta64. Added (passing) tests as well. I had to pepper linspace with datetime specific extra logic in a few places.

Should I submit a PR?

None,
TD(nodatetime_or_obj, out='?'),
'PyUFunc_IsFiniteTypeResolver',
TD(nodatetime_or_obj + times, out='?'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shorter as just noobj (tested to be equivalent on my machine)

Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me - one tiny nit about the ufunc type letters.

@charris charris changed the title isfinite support for datetime64 and timedelta64 ENH: isfinite support for datetime64 and timedelta64 Apr 2, 2019
@charris
Copy link
Member

charris commented Apr 2, 2019

Needs a release note.

@charris
Copy link
Member

charris commented Apr 7, 2019

@debsankha ping. All this needs is a release note (doc/release/1.17.0-notes.rst), and maybe use Eric's shorter type spec.

Debsankha Manik added 2 commits April 7, 2019 14:29
@debsankha
Copy link
Author

OK, done.

@debsankha debsankha closed this Apr 7, 2019
@debsankha debsankha reopened this Apr 7, 2019
@charris charris merged commit 32acbd3 into numpy:master Apr 19, 2019
@charris
Copy link
Member

charris commented Apr 19, 2019

Thanks @debsankha .

@charris charris removed the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label Apr 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add isfinite support for datetime64 and timedelta64
5 participants