Conversation
Signed-off-by: 1fanwang <1fannnw@gmail.com>
|
Thanks for opening a pull request! This pull request has been automatically closed because you currently have 9 open pull requests, which is more than the limit of 3. Due to the increase in pull requests opened by AI bots, and in order to keep the review queue manageable, Apache Arrow limits contributors without repository access to at most 3 concurrently open pull requests. This helps make sure each pull request gets the attention it needs and that work in progress does not go stale. Once one of your other open pull requests has been merged or closed, you are welcome to reopen this one. See also: |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Safe date32/date64 conversions can still overflow or produce wrapped data.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Fixes NumPy datetime and timedelta conversions so unit multipliers are preserved.
Changes:
- Scales temporal values using NumPy unit multipliers.
- Adds regression coverage for dates, durations, strides, masks, and overflow.
| File | Summary |
|---|---|
python/pyarrow/src/arrow/python/numpy_to_arrow.cc |
Implements temporal scaling and overflow handling. |
python/pyarrow/tests/test_array.py |
Adds multiplier and overflow regression tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } else if (cast_options_.allow_time_overflow) { | ||
| out_values[i] = | ||
| static_cast<uint64_t>(in_values[i]) * static_cast<uint64_t>(multiplier); | ||
| } else if (::arrow::internal::MultiplyWithOverflow(in_values[i], multiplier, |

Rationale for this change
Converting a NumPy datetime array with a unit such as
10ssilently changes its timestamps. The example in #39756 turns dates in 2012 into dates in 1974. Duration arrays also lose the multiplier.What changes are included in this PR?
Scale valid temporal values by the NumPy unit multiplier before conversion. Preserve nulls and reject overflow unless unsafe conversion is requested.
Are these changes tested?
Built main and this branch on macOS arm64 with Python 3.12 and NumPy 2.5.3. The regression cases cover timestamps, dates, durations, strides, masks and overflow.
Raw logs
Are there any user-facing changes?
NumPy temporal arrays with unit multipliers now retain their represented values.
This PR contains a "Critical Fix". The old conversion silently produced incorrect timestamps and durations.
Was AI used for this PR?
In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.
PR code and description written by:
Reviewed before submission by: