Talip7/scikit-learn-issues-multilabel huggingface dataset:
Improve the metrics API

### Describe the workflow you want to enable

Currently metrics in scikit-learn have several shortcomings:
- Although it is possible to pass custom metrics as callables, they are called for each pair of inputs. This makes impossible to create custom metrics with the same performance as the existing ones (#22219).
- Every metric has different additional parameters. This requires that methods using them expose the particular parameters or provide a catch all `metric_params` parameter (and not all of them do that).
- Currently, metrics for types other than numeric vectors are not well supported (#15932).
- Some metrics, such as Mahalanobis, depend on the data. There is currently no way to use them in a cross-validation setting without incurring in data leakage (#6915).

### Describe your proposed solution

IMHO, metrics should be:
- Instances of classes, not functions. The additional parameters of the metrics (e.g. the `p` for Minkowski) could then be passed to the metric at creation time. Advantages: no need to homogeneize args, no need for `metric_params`. If the metrics inherit from `BaseEstimator`, the parameters could also be optimized in cross validation.
- They should accept two matrices with the same first dimension and compute all the paired distances. Advantage: no distinction between provided metrics and custom metrics: all of them could be implemented efficiently without multiple calls for each pair of vectors.
- There should be a way to implement an optimized version of pairwise distances. Although a default ok-ish version can be implemented by calling the function several times with the necessary matrices, for some metrics it is possible to have a better implementation. Either there should be a `Metric` ABC class with a default `pairwise` method that can be overriden, or a `singledispatch` function could be made for pairwise distances, so that one can register the implementation for their class. Advantages: again, allowing custom metrics to be implemented efficiently.
- Metrics know their input types. One should be able to provide metrics to compare strings or other custom types, and algorithms that only need to apply a metric, such as k-NN classification, should delegate the validation to the metric.
- (Optional) Some metrics, such as Mahalanobis, depend on the data. These metrics could have a `fit` method, allowing them to be used in cross-validation without incurring in data leakage.

### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
I like the idea of metrics being instances. It's already the case privately in the nearest neighbors and we are currently trying to make them usable more generally (but it's for internal use only). It's not harmless to the sklearn API though because it makes it more verbose to instanciate a model since you first have to instanciate a metric object.

> default pairwise method that can be overriden

Being able to have custom metrics be efficient would be really nice indeed

Labels: New Feature, API, Needs Decision, module:metrics
Improve the metrics API

### Describe the workflow you want to enable

Currently metrics in scikit-learn have several shortcomings:
- Although it is possible to pass custom metrics as callables, they are called for each pair of inputs. This makes impossible to create custom metrics with the same performance as the existing ones (#22219).
- Every metric has different additional parameters. This requires that methods using them expose the particular parameters or provide a catch all `metric_params` parameter (and not all of them do that).
- Currently, metrics for types other than numeric vectors are not well supported (#15932).
- Some metrics, such as Mahalanobis, depend on the data. There is currently no way to use them in a cross-validation setting without incurring in data leakage (#6915).

### Describe your proposed solution

IMHO, metrics should be:
- Instances of classes, not functions. The additional parameters of the metrics (e.g. the `p` for Minkowski) could then be passed to the metric at creation time. Advantages: no need to homogeneize args, no need for `metric_params`. If the metrics inherit from `BaseEstimator`, the parameters could also be optimized in cross validation.
- They should accept two matrices with the same first dimension and compute all the paired distances. Advantage: no distinction between provided metrics and custom metrics: all of them could be implemented efficiently without multiple calls for each pair of vectors.
- There should be a way to implement an optimized version of pairwise distances. Although a default ok-ish version can be implemented by calling the function several times with the necessary matrices, for some metrics it is possible to have a better implementation. Either there should be a `Metric` ABC class with a default `pairwise` method that can be overriden, or a `singledispatch` function could be made for pairwise distances, so that one can register the implementation for their class. Advantages: again, allowing custom metrics to be implemented efficiently.
- Metrics know their input types. One should be able to provide metrics to compare strings or other custom types, and algorithms that only need to apply a metric, such as k-NN classification, should delegate the validation to the metric.
- (Optional) Some metrics, such as Mahalanobis, depend on the data. These metrics could have a `fit` method, allowing them to be used in cross-validation without incurring in data leakage.

### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
> It's not harmless to the sklearn API though because it makes it more verbose to instanciate a model since you first have to instanciate a metric object.

For some commonly used metrics without parameters, a pre-created instance could be made, e.g.:
```python
euclidean = Minkowski(p=2)
```

Labels: New Feature, API, Needs Decision, module:metrics
Improve the metrics API

### Describe the workflow you want to enable

Currently metrics in scikit-learn have several shortcomings:
- Although it is possible to pass custom metrics as callables, they are called for each pair of inputs. This makes impossible to create custom metrics with the same performance as the existing ones (#22219).
- Every metric has different additional parameters. This requires that methods using them expose the particular parameters or provide a catch all `metric_params` parameter (and not all of them do that).
- Currently, metrics for types other than numeric vectors are not well supported (#15932).
- Some metrics, such as Mahalanobis, depend on the data. There is currently no way to use them in a cross-validation setting without incurring in data leakage (#6915).

### Describe your proposed solution

IMHO, metrics should be:
- Instances of classes, not functions. The additional parameters of the metrics (e.g. the `p` for Minkowski) could then be passed to the metric at creation time. Advantages: no need to homogeneize args, no need for `metric_params`. If the metrics inherit from `BaseEstimator`, the parameters could also be optimized in cross validation.
- They should accept two matrices with the same first dimension and compute all the paired distances. Advantage: no distinction between provided metrics and custom metrics: all of them could be implemented efficiently without multiple calls for each pair of vectors.
- There should be a way to implement an optimized version of pairwise distances. Although a default ok-ish version can be implemented by calling the function several times with the necessary matrices, for some metrics it is possible to have a better implementation. Either there should be a `Metric` ABC class with a default `pairwise` method that can be overriden, or a `singledispatch` function could be made for pairwise distances, so that one can register the implementation for their class. Advantages: again, allowing custom metrics to be implemented efficiently.
- Metrics know their input types. One should be able to provide metrics to compare strings or other custom types, and algorithms that only need to apply a metric, such as k-NN classification, should delegate the validation to the metric.
- (Optional) Some metrics, such as Mahalanobis, depend on the data. These metrics could have a `fit` method, allowing them to be used in cross-validation without incurring in data leakage.

### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
I think that would be nice but scikit-learn also requires Cython-level API in nearest neighbors to be CPU and memory efficient so this would not be easy to implement and would require to define a stable Application Binary Interface (ABI) if we want to allow for 3rd party code to provide custom metrics.

Labels: New Feature, API, Needs Decision, module:metrics
Improve the metrics API

### Describe the workflow you want to enable

Currently metrics in scikit-learn have several shortcomings:
- Although it is possible to pass custom metrics as callables, they are called for each pair of inputs. This makes impossible to create custom metrics with the same performance as the existing ones (#22219).
- Every metric has different additional parameters. This requires that methods using them expose the particular parameters or provide a catch all `metric_params` parameter (and not all of them do that).
- Currently, metrics for types other than numeric vectors are not well supported (#15932).
- Some metrics, such as Mahalanobis, depend on the data. There is currently no way to use them in a cross-validation setting without incurring in data leakage (#6915).

### Describe your proposed solution

IMHO, metrics should be:
- Instances of classes, not functions. The additional parameters of the metrics (e.g. the `p` for Minkowski) could then be passed to the metric at creation time. Advantages: no need to homogeneize args, no need for `metric_params`. If the metrics inherit from `BaseEstimator`, the parameters could also be optimized in cross validation.
- They should accept two matrices with the same first dimension and compute all the paired distances. Advantage: no distinction between provided metrics and custom metrics: all of them could be implemented efficiently without multiple calls for each pair of vectors.
- There should be a way to implement an optimized version of pairwise distances. Although a default ok-ish version can be implemented by calling the function several times with the necessary matrices, for some metrics it is possible to have a better implementation. Either there should be a `Metric` ABC class with a default `pairwise` method that can be overriden, or a `singledispatch` function could be made for pairwise distances, so that one can register the implementation for their class. Advantages: again, allowing custom metrics to be implemented efficiently.
- Metrics know their input types. One should be able to provide metrics to compare strings or other custom types, and algorithms that only need to apply a metric, such as k-NN classification, should delegate the validation to the metric.
- (Optional) Some metrics, such as Mahalanobis, depend on the data. These metrics could have a `fit` method, allowing them to be used in cross-validation without incurring in data leakage.

### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
Our scorers are now instances, and with metadata routing some of the issues here are solved. What's left from this issue? Some performance issues?

Labels: New Feature, API, Needs Decision, module:metrics
Improve the metrics API

### Describe the workflow you want to enable

Currently metrics in scikit-learn have several shortcomings:
- Although it is possible to pass custom metrics as callables, they are called for each pair of inputs. This makes impossible to create custom metrics with the same performance as the existing ones (#22219).
- Every metric has different additional parameters. This requires that methods using them expose the particular parameters or provide a catch all `metric_params` parameter (and not all of them do that).
- Currently, metrics for types other than numeric vectors are not well supported (#15932).
- Some metrics, such as Mahalanobis, depend on the data. There is currently no way to use them in a cross-validation setting without incurring in data leakage (#6915).

### Describe your proposed solution

IMHO, metrics should be:
- Instances of classes, not functions. The additional parameters of the metrics (e.g. the `p` for Minkowski) could then be passed to the metric at creation time. Advantages: no need to homogeneize args, no need for `metric_params`. If the metrics inherit from `BaseEstimator`, the parameters could also be optimized in cross validation.
- They should accept two matrices with the same first dimension and compute all the paired distances. Advantage: no distinction between provided metrics and custom metrics: all of them could be implemented efficiently without multiple calls for each pair of vectors.
- There should be a way to implement an optimized version of pairwise distances. Although a default ok-ish version can be implemented by calling the function several times with the necessary matrices, for some metrics it is possible to have a better implementation. Either there should be a `Metric` ABC class with a default `pairwise` method that can be overriden, or a `singledispatch` function could be made for pairwise distances, so that one can register the implementation for their class. Advantages: again, allowing custom metrics to be implemented efficiently.
- Metrics know their input types. One should be able to provide metrics to compare strings or other custom types, and algorithms that only need to apply a metric, such as k-NN classification, should delegate the validation to the metric.
- (Optional) Some metrics, such as Mahalanobis, depend on the data. These metrics could have a `fit` method, allowing them to be used in cross-validation without incurring in data leakage.

### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
I think (if I understand correctly), your proposal seems backward compatible and doesn't affect users who don't use custom metrics.

From a first read, I'm quite happy with your proposal, but I don't know if any of the maintainers at the moment have the capacity to tackle the issue.

So I think if you start a draft PR, ideally with minimal changes at the time, breaking down the work into smaller pieces (for easier review process), we can have a path forward.

Labels: New Feature, API, Needs Decision, module:metrics
Pin dependencies version in CI with lock files with a bot that auto-updates the lockfiles

### Motivation

- avoid CI breakage due to dependencies updates (recent examples: setuptools update, pytest7 update, coverage 6.3 update, very likely others ...)
- avoid painful debugging when a breakage happens to figure out what is the cause of the problem and generally end up after some time that a dependency has changed
- avoid breaking everyone PRs which is confusing for contributers

### Ideas for implementation

- have environment.yml files for each build rather than versions defined in azure/Circle .yml (for the build using build, we can either use  poetry or conda-lock is supposed to support pip section in the environment.yml although I have not managed to get it to work yet)
- have a script using [conda-lock](https://github.com/conda-incubator/conda-lock) to regenerates conda lock files for each build based on their environment.yml
- have a bot that runs this script updates the lock files and creates a PR, it seems like using an approach using GH action like  Hypothesis does could work:
  https://github.com/HypothesisWorks/hypothesis/commit/c96bbc86efbaa46a78bbb075a36869734dde76b9.
- Have this PR merged if CI passes
- only one PR opened at a time to avoid generating many PRs with the same diff. If there is already an open PR (based on a github label for example), push -f  to the PR branch

### Where to start

I'll probably try out this idea on one build first (e.g. one using conda-forge) to test it out, see how that works, and go from there.

cc @thomasjpfan in case you have some thoughs about this or you have already started working on something similar

### Rejected alternatives

In an ideal workd we would "just" use Dependabot but [it can spam forks](https://github.com/dependabot/dependabot-core/issues/2804) with PRs. Numpy has given up on it see for example [this](https://twitter.com/ralfgommers/status/1457023221841149955)

COMMENT:
We should make sure the contributing docs stay up to date with the pinned versions.

Labels: Build / CI
Pin dependencies version in CI with lock files with a bot that auto-updates the lockfiles

### Motivation

- avoid CI breakage due to dependencies updates (recent examples: setuptools update, pytest7 update, coverage 6.3 update, very likely others ...)
- avoid painful debugging when a breakage happens to figure out what is the cause of the problem and generally end up after some time that a dependency has changed
- avoid breaking everyone PRs which is confusing for contributers

### Ideas for implementation

- have environment.yml files for each build rather than versions defined in azure/Circle .yml (for the build using build, we can either use  poetry or conda-lock is supposed to support pip section in the environment.yml although I have not managed to get it to work yet)
- have a script using [conda-lock](https://github.com/conda-incubator/conda-lock) to regenerates conda lock files for each build based on their environment.yml
- have a bot that runs this script updates the lock files and creates a PR, it seems like using an approach using GH action like  Hypothesis does could work:
  https://github.com/HypothesisWorks/hypothesis/commit/c96bbc86efbaa46a78bbb075a36869734dde76b9.
- Have this PR merged if CI passes
- only one PR opened at a time to avoid generating many PRs with the same diff. If there is already an open PR (based on a github label for example), push -f  to the PR branch

### Where to start

I'll probably try out this idea on one build first (e.g. one using conda-forge) to test it out, see how that works, and go from there.

cc @thomasjpfan in case you have some thoughs about this or you have already started working on something similar

### Rejected alternatives

In an ideal workd we would "just" use Dependabot but [it can spam forks](https://github.com/dependabot/dependabot-core/issues/2804) with PRs. Numpy has given up on it see for example [this](https://twitter.com/ralfgommers/status/1457023221841149955)

COMMENT:
I think it will be cleaner to have the environments described in a `environment.yaml`. For incremental PRs, I see:

1. Move a CI job to a `environment.yaml` file without pinning.
2. Pin dependencies and GitHub actions to bump the versions.

Feel free to experiment with one of the CI jobs. 

**Edit**: Dependabot would be preferred but [it can spam forks](https://github.com/dependabot/dependabot-core/issues/2804) with PRs.

Labels: Build / CI
Pin dependencies version in CI with lock files with a bot that auto-updates the lockfiles

### Motivation

- avoid CI breakage due to dependencies updates (recent examples: setuptools update, pytest7 update, coverage 6.3 update, very likely others ...)
- avoid painful debugging when a breakage happens to figure out what is the cause of the problem and generally end up after some time that a dependency has changed
- avoid breaking everyone PRs which is confusing for contributers

### Ideas for implementation

- have environment.yml files for each build rather than versions defined in azure/Circle .yml (for the build using build, we can either use  poetry or conda-lock is supposed to support pip section in the environment.yml although I have not managed to get it to work yet)
- have a script using [conda-lock](https://github.com/conda-incubator/conda-lock) to regenerates conda lock files for each build based on their environment.yml
- have a bot that runs this script updates the lock files and creates a PR, it seems like using an approach using GH action like  Hypothesis does could work:
  https://github.com/HypothesisWorks/hypothesis/commit/c96bbc86efbaa46a78bbb075a36869734dde76b9.
- Have this PR merged if CI passes
- only one PR opened at a time to avoid generating many PRs with the same diff. If there is already an open PR (based on a github label for example), push -f  to the PR branch

### Where to start

I'll probably try out this idea on one build first (e.g. one using conda-forge) to test it out, see how that works, and go from there.

cc @thomasjpfan in case you have some thoughs about this or you have already started working on something similar

### Rejected alternatives

In an ideal workd we would "just" use Dependabot but [it can spam forks](https://github.com/dependabot/dependabot-core/issues/2804) with PRs. Numpy has given up on it see for example [this](https://twitter.com/ralfgommers/status/1457023221841149955)

COMMENT:
> Note that --update was added in https://github.com/conda-incubator/conda-lock/pull/124 so one needs to install from source.

I don't think we absolutely need to use `--update`, I was thinking of recomputing the lock file from scratch i.e. just rerunning the `conda-lock lock` command. It seems like `--update` is useful if you only want to update one package.

This way a PR could update all the dependencies at once, I don't think this is a big problem, and if we run this often (e.g. once a day) it probably won't happen that often that multiple dependencies get updated in the same PR.

Labels: Build / CI