fix: support multi-series X for aligner fit - #514
Open
Saswatsusmoy wants to merge 1 commit into
Open
Conversation
Aligners need X as a list of series (df-list). fit only resolved a single series and routed aligners through the forecaster fit path, so airline and similar inputs failed mtype checks. Allow X_handle/X_dataset as a list of ids, assemble df-list (Series to frame), route object_type=aligner to fit(X), and extend call_method injection the same way. Tests cover the issue path and list inputs.
Contributor
Author
|
one caveat for reviewers / users: a single series (e.g. y_dataset=airline alone) still fails, on purpose. aligners need multiple series, so we return a short error asking for X_handle or X_dataset as a list of at least two ids. the bug was never being able to pass that list and wrong fit routing. multi-series list path is what this pr fixes. |
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
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.
Reference Issues/PRs
Fixes #491
What does this implement/fix? Explain your changes.
Aligners need
Xas a panel / df-list (list of series).fitonly resolved a single series perX_*/y_*slot and routedobject_type=alignerthrough the forecaster-stylefit(y, X=...)path. That produced the df-list type error on a single series (e.g. airline) and a signature clash when onlyXwas set.This change:
X_handle/X_datasetbe a string or a list of idsfit(X)with a clear error on a single series/tablecall_method*_dataset/*_data_handleinjection the same way for multi-series listsShared resolve is used by sync
fitandfit_async. Tests useAlignerNaive(no soft deps).Does your contribution introduce a new dependency? If yes, which one?
No.
What should a reviewer concentrate their feedback on?
X_handle/X_datasetand Series → DataFrame for df-list itemsExecutor.fit(including single flat DataFrame vs MultiIndex panel)call_methodlist injection parity with fit (len >= 2)Any other comments?
make checkis green locally (206 tests). Commit-time pre-commit passed on this branch.PR checklist
For all contributions
make check).docs/source/. n/a (schema/description only on the existingfittool; no new tool)examples/. n/a