fix: normalise loaded handles to PeriodIndex so seasonal forecasters work (#531) - #542
Open
Shashankss1205 wants to merge 1 commit into
Open
fix: normalise loaded handles to PeriodIndex so seasonal forecasters work (#531)#542Shashankss1205 wants to merge 1 commit into
Shashankss1205 wants to merge 1 commit into
Conversation
…work (#531) load_data_source produced a DatetimeIndex with freq "MS" (MonthBegin) for monthly data. Seasonal forecasters (sp>1) coerce the index to a PeriodIndex internally via index.to_period(freq) and raise "<MonthBegin> is not supported as period frequency" — so fit succeeded and predict failed, with no signal at fit time and no in-toolset workaround. This also broke evaluate and demo-fit + handle-update (NB-19) for the same reason. Normalise a regular DatetimeIndex to PeriodIndex at load, matching sktime's demo datasets (which carry a PeriodIndex and work). to_period() is called with no argument so pandas maps the offset to its period alias (MS -> "M"); passing the offset string back would re-raise the same error. No-op for irregular indexes, already-PeriodIndex, or non-datetime indexes. Applied in format_data_handle (the auto-format-on-load path) and in the auto-format-disabled fallback. Verified: split_data, save_data, plot_series, inspect_data all still work on the period-indexed handles. Fixes #531; unblocks NB-19. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #531.
Problem
load_data_sourceproduced aDatetimeIndexwith freq"MS"(MonthBegin) for monthly data. Seasonal forecasters (sp>1) coerce the index to aPeriodIndexinternally viaindex.to_period(freq)and raise<MonthBegin> is not supported as period frequency— so fit succeeded and predict failed, with no signal at fit time and no workaround inside the toolset. The same root cause brokeevaluateon handle data and demo-fit + handle-update interop (NB-19).Fix
Normalise a regular
DatetimeIndextoPeriodIndexat load, matching sktime's demo datasets (which carry aPeriodIndexand work end-to-end). Key detail:to_period()is called with no argument so pandas maps the offset to its period alias (MS→"M"); passing the inferred"MS"string back in re-raises the same error. No-op for irregular indexes, already-PeriodIndex, or non-datetime indexes.Applied in
format_data_handle(the auto-format-on-load path) and in the auto-format-disabled fallback, so it holds regardless ofSKTIME_MCP_AUTO_FORMAT.Testing
tests/test_periodindex_normalization.py(7 tests): the helper across MS/inferred/irregular/period inputs; load yields a PeriodIndex; the exact reported repro (fit(NaiveForecaster(sp=12), y_handle)→predict(horizon=6)) now succeeds; seasonalevaluateon a handle returns real (non-NaN) metrics.split_data,save_data,plot_series,inspect_dataall still work on period-indexed handles (cutoff2021-12, 24/6 split, CSV 30 rows, 2-series plot).Verify over the wire (after server restart)
must return a forecast (previously errored at predict).
🤖 Generated with Claude Code