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.

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

[ENH] test that set_params recognizes unique suffixes as aliases for full parameter string #2931

Merged
merged 20 commits into from Nov 3, 2023

Conversation

fkiraly
Copy link
Collaborator

@fkiraly fkiraly commented Jul 5, 2022

Update: The logic in this PR became obsolete after merging it upstream in scikit-base 0.6.1, see
sktime/skbase#229

The PR still adds the tests in sktime, with a scikit-base version condition, but does not add the logic anymore.


This PR introduces experimental functionality to set_params to enable it to recognize unique suffixes as aliases for full parameter strings.

That is, in the example in #2929, it enables set_params - and, in consequence, the param_grid of grid search - to recognize max_depth instead of forecaster__forecast__estimator__max_depth.

This is primarily meant as convenience sugar for the user of grid search or, more generally, set_params.

Contains tests for both nested parameter setting behaviour, as well as parameter key aliasing in nested composites.

@fkiraly fkiraly added API design API design & software architecture enhancement Adding new functionality labels Jul 5, 2022
@fkiraly
Copy link
Collaborator Author

fkiraly commented Jul 5, 2022

Problem with the first draft: if set_params sets simultaneously an estimator and its parameters, this fails, since the parameter names of a component estimator are not accessible easily before it is being set.

Added tests for this.

@fkiraly fkiraly marked this pull request as ready for review July 6, 2022 22:36
@fkiraly fkiraly requested a review from aiwalter as a code owner July 6, 2022 22:36
@fkiraly fkiraly changed the title [ENH] experimental - set_params to recognize unique suffixes as aliases for full parameter string [ENH] set_params to recognize unique suffixes as aliases for full parameter string Oct 7, 2023
@fkiraly
Copy link
Collaborator Author

fkiraly commented Oct 12, 2023

hm, maybe it is best if this change is made only in scikit-base, that way there is no need for deprecation handling in sktime.

fkiraly added a commit to sktime/skbase that referenced this pull request Oct 25, 2023
…arameter string (#229)

Mirror of sktime/sktime#2931

This PR introduces experimental functionality to `set_params` to enable
it to recognize unique suffixes as aliases for full parameter strings.

That is, in the example in
sktime/sktime#2929, it
enables `set_params` - and, in consequence, the `param_grid` of grid
search - to recognize `max_depth` instead of
`forecaster__forecast__estimator__max_depth`.

This is primarily meant as convenience sugar for the user of grid search
or, more generally, `set_params`.

Contains tests for both nested parameter setting behaviour, as well as
parameter key aliasing in nested composites.
@fkiraly fkiraly changed the title [ENH] set_params to recognize unique suffixes as aliases for full parameter string [ENH] test that set_params recognizes unique suffixes as aliases for full parameter string Oct 26, 2023
@fkiraly fkiraly added the module:base-framework BaseObject, registry, base framework label Oct 26, 2023
@fkiraly
Copy link
Collaborator Author

fkiraly commented Oct 26, 2023

Updated this PR as the change is now in scikit-base 0.6.1.

@fkiraly fkiraly merged commit 97fd4ed into main Nov 3, 2023
46 checks passed
@fkiraly fkiraly deleted the set_params_sugar branch November 3, 2023 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API design API design & software architecture enhancement Adding new functionality module:base-framework BaseObject, registry, base framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant