Skip to content

fix: keep the offset's resolution with an array n and in time_ranges - #308

Merged
José Morales (jmoralez) merged 7 commits into
mainfrom
fix/array-n-resolution
Oct 13, 2026
Merged

José Morales (jmoralez) merged 7 commits into
mainfrom
fix/array-n-resolution

Conversation

@jmoralez

Copy link
Copy Markdown
Contributor

Closes #304.

offset_times with an array n and time_ranges lost the resolution of offsets finer than the times' unit: an array n cast the result back to the input unit, and time_ranges truncated the values, which repeats timestamps (e.g. datetime64[s] starts with "500ms" gave [t, t, t+1s]).

Changes

  • offset_times with an array n now returns the same unit as a scalar n:
    • non-Tick offsets no longer cast back to the input unit, which also removes a pandas FutureWarning about concatenating empty entries.
    • Tick offsets use the finer of the times' and the offset's resolution, as pandas does (e.g. Milli(1000) on [s] times gives [ms]).
    • pyarrow-backed times keep their dtype, as before.
  • time_ranges with datetimes builds each period with offset_times(starts, freq, i) and reorders the result per series, so it agrees with offset_times by construction:
    • it keeps the finer unit instead of repeating values.
    • periods=0 returns an empty Series with the starts' dtype instead of raising.
    • tz-aware starts no longer go through object arrays (50k series x 14 periods with D went from ~4.1s to ~15ms).
  • offset_times with an array n and a Tick offset raises OutOfBoundsTimedelta when n * freq overflows, and OverflowError when an unsigned n doesn't fit in int64, instead of silently wrapping around. This matches a scalar n.

🤖 Generated with Claude Code

Closes #304

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jmoralez
José Morales (jmoralez) marked this pull request as ready for review October 13, 2026 22:25
Copilot AI balanced review requested due to automatic review settings October 13, 2026 22:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused implementation is consistent with scalar behavior and is comprehensively tested.

0 open findings

What changed in this PR

Fixes datetime offset precision loss and overflow handling in pandas-backed operations.

Changes:

  • Preserves finer datetime resolutions for array offsets.
  • Reworks datetime range generation to use offset_times.
  • Adds coverage for precision, overflow, timezone, Arrow, and empty inputs.
File Description
utilsforecast/​processing.py Corrects offset resolution, overflow validation, and range generation.
tests/​test_processing.py Tests the corrected behavior and edge cases.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@nasaul Saul (nasaul) 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.

LGTM. Array n now matches scalar n for numpy and tz-aware times, and time_ranges no longer repeats timestamps with sub-unit freqs.

A few pyarrow issues that already exist on main (not introduced here), could go in a follow-up:

  • pyarrow + sub-unit non-Tick offset raises: out.astype(times.dtype) casts back to the input's arrow unit. Casting to an ArrowDtype with out's unit would fix it.
    t = pd.Series(pd.to_datetime(["2020-01-31 10:00:00"])).astype("timestamp[s][pyarrow]")
    offset_times(t, pd.DateOffset(milliseconds=500), np.array([1]))
    # ArrowInvalid: Casting from timestamp[ms] to timestamp[s] would lose data
  • date32[pyarrow] with a Tick freq gives AttributeError: 'pyarrow.lib.DataType' object has no attribute 'unit' from _dt_unit. Scalar n fails too, so it's only the error message.
  • time_ranges with pyarrow timestamp starts raises ArrowTypeError: No temporal attributes found on object for any freq (except periods=0). Probably the most relevant one, since pyarrow-backed pandas is getting more common.

@jmoralez
José Morales (jmoralez) merged commit 6e22659 into main Oct 13, 2026
22 checks passed
@jmoralez
José Morales (jmoralez) deleted the fix/array-n-resolution branch October 13, 2026 22:39
José Morales (jmoralez) added a commit that referenced this pull request Oct 13, 2026
Drops keep_resolution and the arrow handling in time_ranges, which
now come from offset_times after #308.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

offset_times with an array n truncates offsets finer than the times' resolution

3 participants