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

[SUGGESTION] New Version Release #564

Closed KulikDM closed 1 year ago

KulikDM commented 1 year ago

Hi, thanks for accepting my PR 562 recently. It seems that it was with good foresight as scikit-learn released version 1.3.0 yesterday with that previous decorator now being depreciated and removed. However, while the update is integrated in the repo, the PyPi and Conda packages are still vulnerable. Perhaps a new release?

koaning commented 1 year ago

Hi there. Sorry for the delay. Life with a baby gets in the way sometimes.

I'll make a quick release right now for pypi, hopefully the conda-forge repo picks it up quickly after.

koaning commented 1 year ago

Mhm. There seems to be something breaking. Can't make a super quick patch now. Will have to look into this later.

KulikDM commented 1 year ago

@koaning I can only imagine, sterkte, and thank you for getting back ;)

The issue seems to be from a fix make in sklearn = 1.0.3 where pipeline.Pipeline.fit_transform now raises an AttributeError if the last step of the pipeline does not support fit_transform. Not sure how this can be fixed, but here are the changes made in sklearn.

All the best! And will try help if I can.

koaning commented 1 year ago

Ah wait. Then the fix is relatively simple. I just have to update the object that's used in testing. Will figure out some time to have another look!

KulikDM commented 1 year ago

Hi @koaning, I've found a quick fix that can help get the tests to pass. If you add IGNORE_TESTS = ("test_set_pipeline_step_passthrough", "test_pipeline_raise_set_params_error", "test_set_pipeline_steps") <-- to the test_debug_pipeline_w_sklearn_test.py file.

Hope this is a valid temporary/permanent solution.

koaning commented 1 year ago

@MBrouns at this point I'm actually fine with the aforementioned solution. It's not ideal, but neither is endless waiting for a patch release. Any concerns if I turn off those tests with an XFAIL?

KulikDM commented 1 year ago

Hi @koaning, to clarify it's just this test_set_pipeline_steps test, the other two skipped steps already exist in the testing script.

koaning commented 1 year ago

Thanks for the ping there. I went for the quick-fix you suggested.

I just pushed 0.6.15. Let me know if it fixes your issues.

KulikDM commented 1 year ago

@koaning Thank you so much for being so attentive to this. Really appreciate it! And everything is looking all good my side ;)