search

dask / dask-ml

Scalable Machine Learning with Dask
http://ml.dask.org
BSD 3-Clause "New" or "Revised" License
903 stars 256 forks source link

updates for scikit-learn 1.2.0 #959

Closed mmccarty closed 1 year ago

mmccarty commented 1 year ago
mmccarty commented 1 year ago

Ready for review @jrbourbeau cc @betatim

betatim commented 1 year ago

The changes looks reasonable to me. Only question I have is regarding backwards compatibility and such. Does dask-ml make promises about changes of parameter default values?

mmccarty commented 1 year ago

The changes looks reasonable to me. Only question I have is regarding backwards compatibility and such. Does dask-ml make promises about changes of parameter default values?

No promises that I'm aware of. From my experience, the project seems to follow scikit-learn for API changes. I don't intend on adding backwards compatibility with older versions of scikit-learn in this PR. Others are welcome to follow up.

TomAugspurger commented 1 year ago

In general, my policy was to match the lowest-supported version of scikit-learn, but to pretty aggressively bump that minimum supported version when it became at all challenging to support both versions. If pople are able to upgrade their version of scikit-learn to the latest, then they ought to be able to upgrade dask-ml as well (and vice-versa).

On Wed, Jan 18, 2023 at 8:48 AM Mike McCarty @.***> wrote:

The changes looks reasonable to me. Only question I have is regarding backwards compatibility and such. Does dask-ml make promises about changes of parameter default values?

No promises that I'm aware of. From my experience, the project seems to follow scikit-learn for API changes. I don't intend on adding backwards compatibility with older version of scikit-learn in this PR. Others are welcome to follow up.

— Reply to this email directly, view it on GitHub https://github.com/dask/dask-ml/pull/959#issuecomment-1387193171, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKAOIRSBU7KNZI3DKIRV7LWS77EFANCNFSM6AAAAAAT2XZ5IM . You are receiving this because you are subscribed to this thread.Message ID: @.***>

mmccarty commented 1 year ago

I'm not opposed to this. #957 requires 1.1.3 and is merged. Added #961 to track.

mmccarty commented 1 year ago

@jrbourbeau - Any comments here?

betatim commented 1 year ago

Let's merge this then. I would have clicked the button, but I can't :D