Skip to content

fix: reject a timezone-aware bound when the time column is naive - #331

Open
Muneeb Anjum (horizonbymuneeb) wants to merge 1 commit into
Nixtla:mainfrom
horizonbymuneeb:fix/tz-aware-bound-naive-column
Open

Muneeb Anjum (horizonbymuneeb) wants to merge 1 commit into
Nixtla:mainfrom
horizonbymuneeb:fix/tz-aware-bound-naive-column

Conversation

@horizonbymuneeb

Copy link
Copy Markdown

fill_gaps / id_time_grid accepted a timezone-aware start or end for a timezone-naive pandas time column and silently converted the bound to UTC, dropping the offset.

df = pd.DataFrame({
    "unique_id": "a",
    "ds": pd.date_range("2024-01-02", periods=5, freq="D"),   # naive
    "y": np.arange(5.0),
})
start = pd.Timestamp("2024-01-01", tz="Asia/Tokyo")

fill_gaps(df, "D", start=start)["ds"].iloc[0]
# before: 2023-12-31 00:00:00        (a day early, and every value comes back NaN)
# after:  ValueError: Bound start=Timestamp('2024-01-01 00:00:00+0900', tz='Asia/Tokyo')
#         is timezone-aware but the time column has dtype datetime64[ns], which has no timezone.

Root cause

_determine_bound did bound.tz_convert("UTC").tz_localize(None) for any tz-aware pd.Timestamp, without asking whether the time column has a timezone at all. Converting local midnight in Tokyo to UTC gives 2023-12-31 15:00, which then gets truncated to 2023-12-31 for a daily grid — one period before the date the user wrote. The original rows no longer line up with the grid, so they come back as NaN under the generic "some values were lost during filling" warning. The bound looks usable and the loss looks like a frequency problem, which is what makes it hard to diagnose.

The polars path refuses the same input already, because there is no way to know which zone a naive column is in.

Fix

_validate_bound_pd raises the equivalent ValueError from the pandas branch of id_time_grid, before the column is converted to UTC. Only a pd.Timestamp carrying a timezone on a naive column trips it — "per_serie", "global", date strings and integers are untouched, and a tz-aware column with a tz-aware bound still works as before.

The error message names the argument (start or end) as well as the dtype, since both bounds are validated at the same place.

Tests

tests/test_preprocessing.py gains TestFillGapsTimezoneBounds:

test asserts
test_tz_aware_bound_on_naive_column_raises start and end both raise, parametrized
test_message_names_the_column_dtype the message carries the column dtype
test_naive_bounds_on_naive_column_still_work 6 rows, correct first timestamp, 1 NaN — unchanged
test_tz_aware_column_keeps_working a tz-aware column with a tz-aware bound still builds a grid

Mutation check: with the fix stashed, three of these fail (both parametrizations of the raise test, plus the message test); with the fix, all five pass.

Test results

run result
pytest tests/test_preprocessing.py -k TimezoneBounds 5 passed
pytest tests/ (minus test_evaluation.py, which needs pyspark) 1946 passed, 27 skipped, 5 failed

The 5 failures are pre-existing in this environment and identical before and after the change — test_processing.py polars-version drift (pl.Expr, AttributeError) and a test_validation.py datetime-dtype assertion. Comparing the two failure sets directly gives an empty difference, and the pass count rises by exactly the 5 tests added here.

ruff check is clean on both files. I did not run ruff format across the test file: it is not ruff-formatted at its default width on main either, so reformatting it would bury a 45-line addition in unrelated churn.

Compatibility

This changes behaviour only for input that previously returned a silently wrong grid: a tz-aware bound against a naive column now raises instead. Code that passed such a bound and did not notice the shifting grid will see the error, which is the point. Everything else — naive bounds, tz-aware columns, integer and polars paths — is unchanged.

`fill_gaps` / `id_time_grid` accepted a timezone-aware `start` or `end` for a
timezone-naive pandas time column and silently converted the bound to UTC,
dropping the offset. A bound written as local midnight therefore landed on the
previous day:

    df  ds = 2024-01-02 .. 2024-01-06 (naive)
    start = pd.Timestamp("2024-01-01", tz="Asia/Tokyo")
    fill_gaps(df, "D", start=start)["ds"].iloc[0]   # 2023-12-31

The grid then started a period early and the original values came back as NaN
under the generic "some values were lost" warning, so the bound looked usable and
the loss looked like a frequency problem.

The polars path already refuses this, because there is no way to know which zone
the naive column is in. The pandas path now raises the same `ValueError` before
the column is converted, so both backends agree. A tz-aware column with a tz-aware
bound keeps working, and naive bounds are untouched.
@CLAassistant

CLAassistant commented Nov 4, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

This branch has not been deployed

No deployments
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.

[preprocessing] fill_gaps with pandas silently converts a timezone-aware start/end to UTC when the time column is naive

2 participants