search

koaning / scikit-lego

Extra blocks for scikit-learn pipelines.
https://koaning.github.io/scikit-lego/
MIT License
1.28k stars 118 forks source link

Generalize EstimatorTransformer to multi-output estimators. #526

Closed CarloLepelaars closed 2 years ago

CarloLepelaars commented 2 years ago

EstimatorTransformer will only reshape if either the output shape of a given estimator has length 1 or if the 2nd dimension of the output equals 1. For multi-output estimators the estimator output is returned as is. Solves issue #525

koaning commented 2 years ago

@CarloLepelaars before diving into the code, could you also add a test that confirms the new behavior? If possible, it might also be good to confirm that this component works with the FeatureUnion component.

CarloLepelaars commented 2 years ago

@koaning 👍 Sure, will do!

koaning commented 2 years ago

Any objections against adding a test that uses MultiOutputRegressor?

CarloLepelaars commented 2 years ago

No objections at all! 🙂

Changed the test so it uses MultiOutputRegressor and Ridge. Locally all tests are passing now, but still not sure if random_xy_dataset_multitarget is passed correctly. Not that familiar with pytest.fixture to be honest.

koaning commented 2 years ago

If only there's a website that clearly explains them ;)

https://calmcode.io/pytest-tricks/fixtures.html

CarloLepelaars commented 2 years ago

Aha, awesome! Will review that Calmcode section! 🙏

CarloLepelaars commented 2 years ago

Awesome! This is a great project you guys have going! Looking forward to contribute more in the future.