Convert 0-D arrays in the time converters - #802
Conversation
to_datetime64 and to_timedelta64 raised "TypeError: len() of unsized object" for any 0-D array whose dtype missed the first branch: a 0-D float, int, or the other time type. Only the matching time type worked, because that branch returns before the length check. Both converters now reshape a 0-D input to the length-one array it stands for and unpack the scalar back out, so every rank takes the same path. _array_to_timedelta64's early returns become an if/elif/else to give it a single exit to unpack at.
|
Warning Review limit reached
Next review available in: 33 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5855a1c84a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| elif np.issubdtype(array.dtype, np.datetime64): | ||
| int_array = array.view(np.int64) | ||
| return np.array(int_array).astype("timedelta64[ns]") | ||
|
|
||
| assert np.isreal(array[0]) | ||
| invalid = pd.isnull(array) | ~np.isfinite(array) | ||
| # Need to make copy to 1) not change original array and 2) handle | ||
| # immutable arrays. See #575. | ||
| if np.any(invalid): | ||
| array = np.array(array) | ||
| array[invalid] = 0 | ||
| # inf/NaN complain, salience these types of warnings for this block. | ||
| with np.errstate(divide="ignore", invalid="ignore"): | ||
| out = _float_array_to_ns(array).astype("timedelta64[ns]") | ||
| out[invalid] = _NAT_TIMEDELTA64 | ||
| return out | ||
| out = np.array(array.view(np.int64)).astype("timedelta64[ns]") |
There was a problem hiding this comment.
When the new 0-D path receives a datetime64 whose unit is not nanoseconds, this views the raw unit count and then labels it as nanoseconds. For example, to_timedelta64(np.array(np.datetime64("2020-01-01", "s"))) now returns 1577836800 ns (about 1.58 seconds) rather than the nanosecond-normalized offset from the epoch. Cast the datetime array to datetime64[ns] before viewing its integer representation; the new test currently misses this because it compares against the same incorrect length-one-array path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and worse than the 0-D case alone — the 1-D path had the same bug for every non-ns unit. to_timedelta64(np.array([np.datetime64("2020-01-01", "D")])) returned 18262 ns instead of 18262 days; only an array already in ns was right. Fixed in 0e6fd46 by casting to ns before the view, with a test parametrized over D/s/ms/ns asserting the epoch offset directly rather than against the other rank.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #802 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 164 164
Lines 17829 17835 +6
=========================================
+ Hits 17829 17835 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| degenerate = _is_degenerate(array) | ||
| if degenerate: | ||
| array = array.reshape(1) |
There was a problem hiding this comment.
Too much code; just check if it is degenerate and delete the helper function.
There was a problem hiding this comment.
Done in 0e6fd46 — inlined as degenerate = array.ndim == 0 with a two-line comment, helper deleted.
| degenerate = _is_degenerate(array) | ||
| if degenerate: | ||
| array = array.reshape(1) |
There was a problem hiding this comment.
Done — same inline check, pointing at the note in _array_to_datetime64 rather than repeating it.
to_timedelta64 of a datetime64 array viewed its raw integer and labeled the result nanoseconds, so only an array already in ns was right: a datetime64[D] of 2020-01-01 came back as 18262 ns rather than 18262 days. Casting to ns first makes every unit give the same epoch offset. Also inlines the degenerate check and drops the helper, per review.
1 participant
Description
to_datetime64andto_timedelta64raisedTypeError: len() of unsized objectfor a 0-D array, unless its dtype happened to match the converter's own time type:The empty-array guard is
len(array) == 0, which a 0-D array cannot answer. Only the matching time type worked, because that branch returns before reaching the check. Every one of these has a 1-D equivalent that works fine, so the rank was the only thing standing in the way.Both converters now reshape a 0-D input to the length-one array it stands for and unpack the scalar back out, so every rank takes the same path and a 0-D input converts exactly like
array.reshape(1)would._array_to_timedelta64's early returns become anif/elif/elseto give it a single exit to unpack at.Nothing about 1-D or higher input changes. 2-D input still raises the same
ValueErrorit always did.Found while reviewing #800.
Changelog
dc.to_datetime64anddc.to_timedelta64convert a 0-D array instead of raisingTypeError: len() of unsized object.to_timedelta64normalizes adatetime64array's unit before viewing it as integers, so a non-nanosecond array (e.g.datetime64[s]) is no longer off by a factor of a billion.Checklist
I have (if applicable):