Repository navigation
BUG/API: return numpy scalars instead of ints from datetime64/timedelta64.astype(object) - #31023
jbrockmendel wants to merge 4 commits into
Conversation
a53aabe to
6e58a98
Compare
|
This is along the lines of @seberg's comment, your comment and #18683, glad to see it no longer segfaults/recurses. I don't really mind that you used AI as long as we can talk to a human, thanks for the honest disclosure. Also related to #7619. This definitely needs a release note and is quite a large change. We could sweep it under the rug as a "bugfix", but I think it is bigger than that. How can we best communicate the change to users? How would it affect Pandas? |
|
N.B.: #30985 makes a point that mixing Python and NumPy timedelta/datetime currently works a little bit sometimes on grounds of the path removed here. Not sure it matters but maybe you have a quick thought @jbrockmendel? But, I am positively surprised things work this much, the guard against recursion is correct (and maybe an acceptable regression, even if it returned the value for I am wondering if removing this means that we may need a ufunc(?) to convert, but raise an error probably if it fails? OTOH, considering that (There is one weird(?) possibility, we could in theory make |
Once our minimum-version catches up, it would let us rip out some special-casing we currently have to deal with the current behavior. It's possible to keep the current behavior for coarser resolutions and only change it for a) microsecond cases that are out of bounds for pydatetime and b) finer resolutions. But thats an ugly enough API that i made a judgement call to handle all cases the same. If going the other way is what it takes to get merged though, I'll change it. |
66f66c4 to
24e9f3b
Compare
… astype(object) convert_datetime_to_pyobject and convert_timedelta_to_pyobject returned raw Python ints for values that could not be represented as Python datetime.datetime or datetime.timedelta (sub-microsecond units, out-of-range years, leap seconds, overflowing timedelta days). This caused arr.astype(object) to silently produce integers, breaking roundtrips and surprising users. Replace those PyLong_FromLongLong fallbacks with PyArray_Scalar calls that return proper numpy.datetime64 / numpy.timedelta64 scalars. Because getitem now returns numpy scalars in those cases, int() and float() on datetime64/timedelta64 scalars would infinitely recurse through gentype_int -> array_int -> getitem -> gentype_int. Break the recursion by giving the two scalar types their own tp_as_number with nb_int/nb_float that raise TypeError, consistent with how int(datetime.datetime(...)) and int(datetime.timedelta(...)) already behave. Closes numpy#12550 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
24e9f3b to
8b45cb4
Compare
Conflicts: - arrays.datetime.rst: main added .item() examples showing NaT -> None and ns -> int. Replaced with the numpy-scalar results, keeping the timedelta-NaT case main added coverage for. - test_scalarmath.py: main switched to an explicit 'D' unit. Kept that, with the updated np.equal(NaT, None) assertion. Also drops the comments describing the ==/np.equal inconsistency for NaT, in test_scalarmath.py and gentype_richcompare, which this branch resolves. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01YF8EFz6MfYRc2whjZEiDtu
…oving Values with no datetime.datetime or datetime.timedelta representation (sub-microsecond units, Y/M, generic units, out-of-range years, leap seconds, overflowing timedelta days) converted to an int before. Rather than raising TypeError immediately, keep returning the integer with a DeprecationWarning; everything else still raises. nb_int/nb_float read obval off the scalar rather than going through getitem, so the recursion the previous TypeError guarded against cannot arise. Also expand the release note to cover the breakages it was silent on: NaT no longer converting to None, .tolist() returning numpy scalars, and arithmetic mixing numpy and Python datetime types now raising. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01YF8EFz6MfYRc2whjZEiDtu
test_datetime64_item_int64_min_edge_case asserted that .item() on a datetime64 near int64 min returns a Python int, which no longer holds. Assert the datetime64 round-trip instead, and reach the conversion that used to overflow through str(), which still goes through days_to_yearsdays. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01YF8EFz6MfYRc2whjZEiDtu
|
Thanks for your patience. Just updated.
I'd advocate against that since it would mean a different type depending on your resolution and whether you are OOB for stdlib datetime.
IIUC you're suggesting that
I don't have any bright ideas. Could just wait for a major release I guess. |
cc @seberg
Claude wrote this, I reviewed it manually. But my C-fu is weak, so I'm skirting the bounds of socially-acceptable AI use. My feelings won't be hurt if reviewers think I'm over the line.
TLDR:
dt64array.astype(object)[0]give np.datetime64 objects instead of integers (the point of #12550).Making
float(d64obj)andint(dt64obj)raise is just because claude says its necessary to avoid recursion and a segfault.