Skip to content

GH-39756: [Python] Preserve NumPy temporal unit multipliers - #51480

Closed
1fanwang wants to merge 1 commit into
apache:mainfrom
1fanwang:1fannnw/numpy-datetime-unit-multiples
Closed

1fanwang wants to merge 1 commit into
apache:mainfrom
1fanwang:1fannnw/numpy-datetime-unit-multiples

Conversation

@1fanwang

@1fanwang 1fanwang commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Rationale for this change

Converting a NumPy datetime array with a unit such as 10s silently 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
python -c 'import numpy as np, pyarrow as pa; a = np.arange("2012-01-01", "2012-01-01T00:01", dtype="datetime64[10s]"); print(pa.array(a)[0].as_py(), pa.array(a)[-1].as_py())'

Before:
1974-03-15 00:00:00 1974-03-15 00:00:05
After:
2012-01-01 00:00:00 2012-01-01 00:00:50

python -m pytest pyarrow/tests/test_array.py -k temporal_unit_multiplier -q
Before: 88 failed, 2 passed, 328 deselected in 2.76s
After: 90 passed, 328 deselected in 0.10s

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:

  • Human
  • AI

Reviewed before submission by:

  • Human
  • AI
  • Not reviewed

Signed-off-by: 1fanwang <1fannnw@gmail.com>
Copilot AI lite review requested due to automatic review settings September 24, 2026 05:50
@github-actions

Copy link
Copy Markdown

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:

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 High severity

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,
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.

2 participants