Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[BUG] fix get_fitted_params in case of vectoriztion for forecasters, transformers #4105

Merged
merged 2 commits into from Jan 23, 2023
Merged

[BUG] fix get_fitted_params in case of vectoriztion for forecasters, transformers #4105

merged 2 commits into from Jan 23, 2023

Conversation

fkiraly
Copy link
Collaborator

@fkiraly fkiraly commented Jan 13, 2023

This PR fixes an unreported bug that was noticed here: #4101 (reply in thread)

The bug causes only the diagonal combinations of rows/columns to be present as string keyable values in the fitted params dict in case of vectorized forecasters and transformers.

What should be present is the cartesian product, not the diagonal. This is solved by correctly using product instead of zip that was used accidentally.

Also fixes an issue (not necessarily bug) with the convention of constructing the strings - they should now always be a string that evaluates to a correct data frame loc access command, which previously was not the case for str keys (as that would eat away the quotes in str coercion).

@fkiraly fkiraly added implementing framework Implementing or improving framework for learning tasks, e.g., base class functionality module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting module:transformations transformations module: time series transformation, feature extraction, pre-/post-processing bugfix Fixes a known bug or removes unintended behavior labels Jan 13, 2023
@fkiraly fkiraly merged commit 90e54f0 into main Jan 23, 2023
@fkiraly fkiraly deleted the fix_vectorize_get_fitted_params branch January 23, 2023 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bugfix Fixes a known bug or removes unintended behavior implementing framework Implementing or improving framework for learning tasks, e.g., base class functionality module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting module:transformations transformations module: time series transformation, feature extraction, pre-/post-processing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@fkiraly