8000 Make better common test for `set_output` · Issue #24931 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

Make better common test for set_output #24931

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
Closed
glemaitre opened this issue Nov 15, 2022 · 0 comments · Fixed by #24932
Closed

Make better common test for set_output #24931

glemaitre opened this issue Nov 15, 2022 · 0 comments · Fixed by #24932
Assignees
@glemaitre
Labels
Blocker
Milestone
1.2

Comments

@glemaitre
Copy link
Member

We have a common test checking that set_output lead to the right results. However, there are some side-effect that we did not anticipate and that are not covered right now.

set_config

One can require a pandas output with set_config:

import sklearn

sklearn.set_config(transform_output="pandas")

while the common test is setting the estimator using set_output, set_config (and config_context) will set the output of the nested estimators. This is something that is not tested and leads to failures as shown in #24923.

We need to add a common test with the context manager to ensure that transformers with nested transformer(s) are still working as expected.

Undefined behaviour

I did not yet make a check but I think that we should make sure to define the following behaviour:

  • request pandas output without providing dataframe at fit and transform
  • request pandas output without providing dataframe at transform
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@glemaitre glemaitre

Labels
Blocker
Projects
None yet
Milestone
1.2
Development

Successfully merging a pull request may close this issue.

MAINT test globally setting output via context manager glemaitre/scikit-learn
1 participant
@glemaitre
0