Failing mlr diagnostic script test

[bouweandela]

The following test is failing:

FAILED tests/integration/diag_scripts/mlr/test_custom_sklearn_functions.py::test_safe_tags_no_get_tags[estimator4-None-expected_results4] - AssertionError: assert {'X_types': [...': False, ...} == {'X_types': [...': False, ...}
  Omitting 18 identical items, use -vv to show
  Left contains 1 more item:
  {'array_api_support': False}
  Full diff:
    {
     'X_types': ['2darray'],
     '_skip_test': False,
     '_xfail_checks': False,
     'allow_nan': False,
  +  'array_api_support': False,
     'binary_only': False,
     'multilabel': False,
     'multioutput': False,
     'multioutput_only': False,
     'no_validation': False,
     'non_deterministic': False,
     'pairwise': False,
     'poor_score': False,
     'preserves_dtype': [<class 'numpy.float64'>],
     'requires_fit': True,
     'requires_positive_X': False,
     'requires_positive_y': False,
     'requires_y': False,
     'stateless': False,
    }

see here.

[remi-kazeroni]

This seems to come from the latest Scikit-learn 1.3.0, released 3 days ago, and that we are getting in our envs.

[bouweandela]

Any suggestion for updating our code so it works with the latest version?

[remi-kazeroni]

Any suggestion for updating our code so it works with the latest version?

See #3271 for a tentative fix

[remi-kazeroni]

Reopening the issue to give @schlunma a chance to investigate deeper this change of scikit-learn version beyond what was done to fix the test for the upcoming release.

[valeriupredoi]

thanks for the care about this, Remi! @schlunma something to help you forget about all that fun you had on your vacation 😁 🏖️

[valeriupredoi]

just a heads up that the change in #3273 is not backwards compatible for scikit-learn <1.3.0, as we can see from the failed conda lock test

[bouweandela]

When I checked it seemed to be, but it looks like I messed up 🤦

[valeriupredoi]

not a worry, Bouwe, Manu will have this shipshape when he's off the cruise ship 🚢

[remi-kazeroni]

I was just playing around with various versions of scikit-learn from 1.2.0 until 1.3.0 (all Python 3.11 compatible) and tests pass fine locally if we had:

from sklearn.utils._tags import (
    _DEFAULT_TAGS,
)

instead of a copy in our own code. At first sight, this does not seem to be a very intrusive change but I don't know much about these mlr diagnostics...

[bouweandela]

That would mean importing a rather internal bit of scikit-learn, definitely not recommended because it may be removed at any point without prior warning.

[schlunma]

Thanks everyone for fixing this! I think #3273 does the right job.

I agree with @bouweandela that we should not use private functionalities of sklearn (that's why I wrote this module in the first place). To make the test backwards-compatible, I propose to add a another mock class that simply exposes the _DEFAULT_TAGS. See #3285 for a suggestion of this.