From f511c82f75fcc518eb71f86ede592202faab8c53 Mon Sep 17 00:00:00 2001 From: Muneeb Anjum Date: Wed, 4 Nov 2026 09:40:49 +0500 Subject: [PATCH] fix: reject a timezone-aware bound when the time column is naive `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. --- tests/test_preprocessing.py | 45 ++++++++++++++++++++++++++++++++++ utilsforecast/preprocessing.py | 24 ++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/tests/test_preprocessing.py b/tests/test_preprocessing.py index 986b65a..f9ef030 100644 --- a/tests/test_preprocessing.py +++ b/tests/test_preprocessing.py @@ -152,6 +152,51 @@ def get_integer_freqs(): # --- Basic fill_gaps tests --- +class TestFillGapsTimezoneBounds: + """A timezone-aware bound is only usable when the time column has a timezone. + + The pandas path used to convert such a bound to UTC and drop the offset, so a + bound written as local midnight silently landed on the previous day: the grid + started one period early and the original values were then reported lost. The + polars path already raised, so this keeps the two backends in agreement. + """ + + @pytest.fixture + def naive_df(self): + return pd.DataFrame( + { + "unique_id": "a", + "ds": pd.date_range("2024-01-02", periods=5, freq="D"), + "y": np.arange(5.0), + } + ) + + @pytest.mark.parametrize("bound_name", ["start", "end"]) + def test_tz_aware_bound_on_naive_column_raises(self, naive_df, bound_name): + bound = pd.Timestamp("2024-01-01", tz="Asia/Tokyo") + with pytest.raises(ValueError, match="is timezone-aware but the time column"): + fill_gaps(naive_df, freq="D", **{bound_name: bound}) + + def test_message_names_the_column_dtype(self, naive_df): + bound = pd.Timestamp("2024-01-01", tz="Asia/Tokyo") + with pytest.raises(ValueError) as excinfo: + fill_gaps(naive_df, freq="D", start=bound) + assert str(naive_df["ds"].dtype) in str(excinfo.value) + + def test_naive_bounds_on_naive_column_still_work(self, naive_df): + result = fill_gaps(naive_df, freq="D", start=pd.Timestamp("2024-01-01")) + assert len(result) == 6 + assert result["ds"].iloc[0] == pd.Timestamp("2024-01-01") + assert result["y"].isna().sum() == 1 + + def test_tz_aware_column_keeps_working(self, naive_df): + tz_df = naive_df.assign(ds=naive_df["ds"].dt.tz_localize("Asia/Tokyo")) + result = fill_gaps( + tz_df, freq="D", start=pd.Timestamp("2024-01-01", tz="Asia/Tokyo") + ) + assert len(result) == 6 + + class TestFillGapsBasic: """Basic tests for fill_gaps function.""" diff --git a/utilsforecast/preprocessing.py b/utilsforecast/preprocessing.py index f0fbd02..7cfde95 100644 --- a/utilsforecast/preprocessing.py +++ b/utilsforecast/preprocessing.py @@ -39,6 +39,28 @@ def _determine_bound(bound, freq, times_by_id, agg) -> np.ndarray: return out +def _validate_bound_pd( + bound: Union[str, int, date, datetime], + name: str, + times: pd.Series, +) -> None: + """Reject a timezone-aware bound for a timezone-naive column. + + The polars path already refuses this, because there is no way to know which zone the naive + column is in. The pandas path instead converted the bound to UTC and dropped the offset, so + a bound written as local midnight landed on the previous day and the grid started off by + one period -- the values were then reported as lost rather than the bound as unusable. + """ + if not isinstance(bound, pd.Timestamp) or bound.tz is None: + return + if times.dt.tz is not None: + return + raise ValueError( + f"Bound {name}={bound!r} is timezone-aware but the time column " + f"has dtype {times.dtype}, which has no timezone." + ) + + def _validate_bound_pl( bound: Union[str, int, date, datetime], name: str, @@ -198,6 +220,8 @@ def id_time_grid( # such as MS = 'Month Start' -> 'M', YS = 'Year Start' -> 'Y' freq = freq[0] delta: Union[np.timedelta64, int] = np.timedelta64(n, freq) + _validate_bound_pd(start, "start", df[time_col]) + _validate_bound_pd(end, "end", df[time_col]) if df[time_col].dt.tz is not None: df = df.copy(deep=False) df[time_col] = df[time_col].dt.tz_convert("UTC").dt.tz_localize(None)