Skip to content

FIX Add support for scikit-learn 1.8#360

Open
dherrera1911 wants to merge 2 commits intoscikit-learn-contrib:masterfrom
dherrera1911:check-xy-fix
Open

FIX Add support for scikit-learn 1.8#360
dherrera1911 wants to merge 2 commits intoscikit-learn-contrib:masterfrom
dherrera1911:check-xy-fix

Conversation

@dherrera1911
Copy link

@dherrera1911 dherrera1911 commented Mar 10, 2026

PR description at a high level:

Support scikit-learn 1.8, that introduced a breaking change. Maintain compatibility with versions <=1.7.

Fixes #359

Describe changes:

In scikit-learn 1.8, the parameter force_all_finite in the functions check_array() and check_X_y() was deprecated, and replaced for ensure_all_finite.

The function _utils.py::check_input() uses the parameter force_all_finite, making the package fail as described in #359.

In this PR, we introduce the local wrapper functions _check_array() and _check_X_y() inside of _utils.py. These wrappers work like their scikit-learn counterparts, but are able to take both force_all_finite and ensure_all_finite as inputs. Now _utils.py calls the wrappers instead of directly calling the functions.

Outstanding questions:

For transparency I mention that the original version of this PR was generated by a coding agent. After reviewing it, I concluded that the approach is sound. I did modify it considerably to make it simpler.

I have not tested this solution thoroughly, but I can run the relevant parts of the testing suite.

@dherrera1911
Copy link
Author

dherrera1911 commented Mar 10, 2026

I ran the tests in test/test_utils.py, and everything seems to be working good. Actually, the current version of metric-learn was throwing errors in these tests even before it broke, because of scikit-learn deprecation warnings.

For reference, this branch passes all tests with scikit-learn 1.8, while main fails a lot of tests.

Interestingly, when using scikit-learn 1.7, both this branch and main throw a bunch of errors saying e.g. FAILED test/test_utils.py::test_check_tuples_valid_tuple_size[2] - assert 4 == 0. This is because in both cases, the pattern below

  with warnings.catch_warnings(record=True) as record:
    check_input(tuples, type_of_inputs='tuples', preprocessor=None)
  assert len(record) == 0

I looked into one of these tests, and as I supposed, it was a FutureWarning from scikit-learn, warning about ensure_all_finite:

is catching `scikit-learn` warnings:

sklearn/utils/deprecation.py:132: FutureWarning: 'force_all_finite' was renamed to 'ensure_all_finite' in 1.6 and will be removed in 1.8.
warnings.warn(


In sum, it the current PR makes `metric-learn` work with `scikit-learn` 1.8, at least in the tests I ran and the use cases I tested for my own work.

@dherrera1911 dherrera1911 changed the title FIX TypeError bug from _util.py::check_input() FIX TypeError bug from scikit-learn 1.8 breaking change Mar 11, 2026
@dherrera1911 dherrera1911 changed the title FIX TypeError bug from scikit-learn 1.8 breaking change FIX Add support for scikit-learn 1.8 Mar 13, 2026
@perimosocordiae
Copy link
Contributor

Thanks! I had to manually enable the CI to run on this PR, but it started now.

else:
def vector_norm(X):
return np.linalg.norm(X, axis=1)
import numpy as np
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused why this line is showing up as part of the diff. Maybe a line-ending related change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeError: check_X_y() got an unexpected keyword argument 'force_all_finite'

2 participants