-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
ENH: allow nan -> NaT casting #11851
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
Conversation
* it is now possible to cast from np.nan to the datetime64 "not-a-time" NaT object
# multidimensional non-zero NaN to NaT | ||
(np.ones((5, 3), dtype="M8[us]"), [1, 1]), | ||
# and for generic datetime64 | ||
(np.ones((5, 3), dtype="M8"), [2, 1]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: indent the lines within the list
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a reasonable approach.
See comments on issue - I'm not sure this is useful
Shall we close this and the corresponding issue as wontfix then? |
If we make it wontfix (which it seems like we should, reading these issues) should we still try adding a warning here if someone tries to convert nan to datetime? I kind of think so, since probably a lot of code does this, which is unsafe, and it's easy for them to fix by using |
Also, it appears to me the "real" problem here is that the get/set conversion code is mismatched from the dtype_transfer conversion code. This is a common problem in numpy I've come across before, for instance in #6053. This PR only fixes up the get/set code, and does not change the dtype-transfter casting code which happens here. It looks like the get/set code currently disallows casting from float in all cases, yet the dtype_transfer code allows it in all cases. Consider in master: >>> np.array([1.0]).astype("M8[ns]")
array(['1970-01-01T00:00:00.000000001'], dtype='datetime64[ns]')
>>> np.array([1.0], dtype="M8[ns]")
ValueError: Could not convert object to NumPy datetime I suggest that the "correct" behavior is that both code-paths raise an error when given a float. Based on the get/set code, it seems to me this was the original intention anyway. |
An unsafe casting work-around that allows the assignment originally requested is described in the linked issue, so this is probably less critical now. Furthermore, it seems my proposed approach here is the opposite of what core dev reviewers want. Perhaps opening the PR was useful to reach that conclusion, but I'll close this attempt now. |
Fixes #8780
np.nan
can now be assigned intonp.datetime64
generic and unit-specific arrays and will be cast tonp.datetime64('NaT')
.I haven't yet changed the result of
np.can_cast()
for this scenario though--perhaps feedback would be useful before altering that behavior as well.There's probably a more elegant way to draft the unit test.