Skip to content

fix: support pyarrow-backed dates in offset_times and time_ranges - #303

Merged
José Morales (jmoralez) merged 12 commits into
mainfrom
fix/pyarrow-time-offsets
Oct 14, 2026
Merged

José Morales (jmoralez) merged 12 commits into
mainfrom
fix/pyarrow-time-offsets

Conversation

@jmoralez

@jmoralez José Morales (jmoralez) commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

What

  • offset_times and time_ranges convert pyarrow-backed dates (naive and tz-aware timestamps, date32/date64) to numpy-backed ones, reuse the existing offset logic and cast the result back to the original arrow dtype.
  • Results match the numpy-backed input. When the offset is finer than the timestamp unit, offset_times returns the finer unit, as it does for numpy-backed times.
  • Offsetting date32/date64 values to times that aren't at midnight (e.g. sub-daily frequencies) raises a ValueError instead of silently dropping the time. date64 inputs that already have a time part raise a separate ValueError that points at the input.
  • Adds tests for these dtypes across several frequencies, scalar and array n, Series and Index inputs, nulls, make_future_dataframe and backtest_splits.

Why

Both functions raised ArrowTypeError: No temporal attributes found on object. for pyarrow-backed dates (and offset_times with an array n failed for date32), which broke make_future_dataframe, backtest_splits, future_exog_to_historic, and fourier and trend with h > 0 for those inputs (time_features still fails on them for an unrelated reason, handled in #298). pyarrow can't do calendar arithmetic itself, so the pandas offsets are applied on numpy-backed dates.

Fixes #309

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t import

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 12, 2026 20:47
Copilot AI balanced review requested due to automatic review settings October 12, 2026 20:47

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.

🟡 Changes recommended

Array offsets can silently truncate subsecond results before Arrow dtype restoration.

2 open findings
What changed in this PR

Adds pyarrow-backed date support to pandas time-offset utilities.

Changes:

  • Converts Arrow dates for offset calculations and restores Arrow dtypes.
  • Rejects non-midnight results for Arrow date types.
  • Adds Arrow timestamp/date integration tests.
File Description
utilsforecast/​processing.py Implements Arrow date conversion and offset handling.
tests/​test_processing.py Tests Arrow-backed temporal operations and integrations.

🧠 Review effort: Balanced


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

Comment thread utilsforecast/processing.py Outdated
Comment thread tests/test_processing.py
…date64

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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 conversion logic is focused and supported by comprehensive tests for the affected behavior.

0 open findings

2 resolved since last review

🧠 Review effort: Balanced

Comment thread utilsforecast/processing.py Outdated
Comment thread tests/test_processing.py
Comment thread utilsforecast/processing.py Outdated
Comment thread utilsforecast/processing.py Outdated
Comment thread utilsforecast/processing.py Outdated
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>
…illiseconds

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>
@nasaul

Copy link
Copy Markdown
Contributor

The offset logic looks correct. A few things before merging:

PR description. The description says this fixes "the feature engineering functions with h > 0" for pyarrow-backed dates, but time_features still fails on timestamp[ns][pyarrow] and date32[pyarrow]. For example, time_features(df, freq="D", features=["month"], h=3) raises AttributeError: 'Index' object has no attribute 'dt', and so does h=0. That bug is older than this PR and comes from the pd.Index wrapping in feature_engineering.py. fourier and trend do work now. Please narrow the claim to those, or fix time_features here too.

time_ranges converts once per period (processing.py:599). [offset_times(starts, freq, i) for i in range(periods)] converts the same arrow starts to numpy and back periods times, and repeats the date midnight check each time. With tz-aware arrow times, make_future_dataframe with h=48 does the cast, tz_localize, tz_convert and the cast back 48 times. Could we convert starts once, build every period on the numpy-backed values, and cast back once at the end?

Midnight error blames the offset (processing.py:471-477). The check only runs on the result, so a date64 input that already has a time part fails with "Cannot represent the offset times ... aren't at midnight", even though the offset didn't add that time part (e.g. a value 500ms past midnight offset by "D"). Checking the input separately, or wording the message so it covers both cases, would make it less misleading. test_offset_times_pyarrow_date64_not_at_midnight only matches the generic text, so it should be tightened along with the message.

Duplicated tz conversion (processing.py:459-462). The Series branch (utc.dt.tz_localize("UTC").dt.tz_convert(tz)) and the Index branch (utc.tz_localize("UTC").tz_convert(tz)) do the same thing. Converting once on pd.DatetimeIndex(utc) and re-wrapping as a Series when needed would stop the two from drifting apart. Minor.

…uts with a time part

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d drop unrelated test reformatting

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jmoralez
José Morales (jmoralez) merged commit 8576c45 into main Oct 14, 2026
21 checks passed
@jmoralez
José Morales (jmoralez) deleted the fix/pyarrow-time-offsets branch October 14, 2026 23:25
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 and time_ranges fail with pyarrow-backed times

3 participants