Talip7/scikit-learn-issues-multilabel huggingface dataset:
FastOPTICS in `sklearn.cluster`

### Describe the workflow you want to enable

Is there a roadmap to add the FastOPTICS algorithm, [1], to the `sklearn.cluster` code base that already supports OPTICS?

[1] 2013, J. Schneider, M. Vlachos, _Fast Parameterless Density-based Clustering via Random Projections 

### Describe your proposed solution

The solution would be to combine what has already been done for the base OPTICS algorithm, combined with the existing code base for random projections and the Johnson-Lindenstrauss bound in `sklearn.random_projection`, to implement FastOPTICS.

The [implementation](https://github.com/elki-project/elki/blob/master/elki-clustering/src/main/java/elki/clustering/optics/FastOPTICS.java) in the data mining library ELKI (albeit in Java) could be used as an inspiration.

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

_No response_

### Additional context

_No response_

COMMENT:
Hi @adrinjalali,

Sorry for the GPL constraint, I am not familiar with ELKI and didn't check the license.

As for the selection criteria, I guess the one that the paper doesn't cut is having 200+ citations, am I right? From a few experiments with ELKI, I would say it really speeds OPTICS up, especially when the data is in very high dimensions, but I also noticed how the paper doesn't provide a quantitative analysis on how close it actually is to the base OPTICS in different data scenarios

Labels: New Feature, Needs Decision, module:cluster
FastOPTICS in `sklearn.cluster`

### Describe the workflow you want to enable

Is there a roadmap to add the FastOPTICS algorithm, [1], to the `sklearn.cluster` code base that already supports OPTICS?

[1] 2013, J. Schneider, M. Vlachos, _Fast Parameterless Density-based Clustering via Random Projections 

### Describe your proposed solution

The solution would be to combine what has already been done for the base OPTICS algorithm, combined with the existing code base for random projections and the Johnson-Lindenstrauss bound in `sklearn.random_projection`, to implement FastOPTICS.

The [implementation](https://github.com/elki-project/elki/blob/master/elki-clustering/src/main/java/elki/clustering/optics/FastOPTICS.java) in the data mining library ELKI (albeit in Java) could be used as an inspiration.

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

_No response_

### Additional context

_No response_

COMMENT:
@adrinjalali what's our criteria? (beginner here trying to learn)

Labels: New Feature, Needs Decision, module:cluster
FastOPTICS in `sklearn.cluster`

### Describe the workflow you want to enable

Is there a roadmap to add the FastOPTICS algorithm, [1], to the `sklearn.cluster` code base that already supports OPTICS?

[1] 2013, J. Schneider, M. Vlachos, _Fast Parameterless Density-based Clustering via Random Projections 

### Describe your proposed solution

The solution would be to combine what has already been done for the base OPTICS algorithm, combined with the existing code base for random projections and the Johnson-Lindenstrauss bound in `sklearn.random_projection`, to implement FastOPTICS.

The [implementation](https://github.com/elki-project/elki/blob/master/elki-clustering/src/main/java/elki/clustering/optics/FastOPTICS.java) in the data mining library ELKI (albeit in Java) could be used as an inspiration.

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

_No response_

### Additional context

_No response_

COMMENT:
Hi @Veghit,

I think they're listed [here](https://scikit-learn.org/stable/faq.html#id19)

Labels: New Feature, Needs Decision, module:cluster
Criteria for fixing connectivity in agglomerative clustering

### Describe the workflow you want to enable

Hi,

Great work on maintaining sklearn!

The current [fix_connectivity](https://github.com/scikit-learn/scikit-learn/blob/2119490c10675839e546abfedba75d30bfbdc8ad/sklearn/cluster/hierarchical.py#L31) function 'fixes' a disjoint graph by having only **one** connected component. Could you please let me know the reason this is necessary if the number of clusters >= number of connected components?

### Describe your proposed solution

Only fix the connectivity graph if the number of clusters < number of connected components. Please correct me if I am wrong in my assumption that this solution will enforce 'hard' cannot-link constraint in agglomerative (ward) clustering. 

cc: @GaelVaroquaux 

### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
If I remember correctly (but it's been a while), the code builds the full agglomerative tree first and cuts it after. I do not remember whether it is just an optimization / simplification of the code, or if there is a fundamental reason.

But this means that, if I remember correctly, the code will fail with a non-connected graph even if the number of clusters is larger than the number of connected components.

This might very well be a fundamental problem: if you have 4 connected components and you want 6 clusters, how to you decide which connected component you cut in two vs which you do not cut? It's a ill-defined choice that is unrelated to the criterion used for agglomerative clustering. Hence I do not think that your proposal can be implemented in a good way.

Labels: New Feature, Needs Triage
Criteria for fixing connectivity in agglomerative clustering

### Describe the workflow you want to enable

Hi,

Great work on maintaining sklearn!

The current [fix_connectivity](https://github.com/scikit-learn/scikit-learn/blob/2119490c10675839e546abfedba75d30bfbdc8ad/sklearn/cluster/hierarchical.py#L31) function 'fixes' a disjoint graph by having only **one** connected component. Could you please let me know the reason this is necessary if the number of clusters >= number of connected components?

### Describe your proposed solution

Only fix the connectivity graph if the number of clusters < number of connected components. Please correct me if I am wrong in my assumption that this solution will enforce 'hard' cannot-link constraint in agglomerative (ward) clustering. 

cc: @GaelVaroquaux 

### Describe alternatives you've considered, if relevant

_No response_

### Additional context

_No response_

COMMENT:
> the code builds the full agglomerative tree first and cuts it after

I see. Thanks for the info!

Labels: New Feature, Needs Triage
RFC: Breaking Changes for Version 2

A while ago we talked about the possibility of a version 2 with breaking changes. Specifically, this came up in the context of SLEP006: metadata routing, but there are other things we have wanted to break which we can do all in a single release. In this issue we can talk about the possibility of such a release, and maybe a few things we'd like to include in it.

I'll try to summarize the challenges of keeping backward compatibility (BC) in the context of SLEP006:

- `MetaEstimator(Estimator()).fit(X, y, sample_weight)` will raise according to SLEP006 since `sample_weight` is not requested by any object. For backward compatibility, during the transition period, it'll only warn and assume `sample_weight` is requested by `Estimator()`.
- What should the behavior of `GridSearchCV(Estimator(), scorer=scorer).fit(X, y, sample_weight)` be during the transition period? If we keep backward compatibility, it should warn (whatever the warning be), route `sample_weight` to `Estimator`, but not to the `scorer`. And that's only the case because we're keeping BC. From the user's perspective, it's very weird.
- `Pipeline` ATM in certain methods like `fit`, `fit_transform`, `predict`, routes `*_params` to the `fit/etc` of the last estimator only. And `decision_function` and `transform`, `score_samples` have no routing at all. Keeping this BC, proves to be challenging, and a bit nasty. ref: #24270
- In quite a few meta-estimators, we check if the sub-estimator has `sample_weight` in the signature of `fit`, and we pass the given sample weight to it if that's the case, and not if that's not the case. With routing, we would have a much better idea of when to pass sample weight and when not, but if we're keeping BC, it's really challenging to see if we should forward sample weights or not. Both AdaBoost (#24026) and Bagging (#24250) have been very challenging, to the point that we might end up having to check if a sub-estimator is a meta-estimator itself or not, and check signature of sub-estimator's `fit`, and check the given routing info to keep BC.

I'm pretty sure there are other issues we haven't foreseen, and they'll come up as we work on this project. We thought it'd be easy once we get to this point, but BC has proven to make it very tricky and has stalled the progress.

All of the above is adding substantial complexity which is probably error prone, and buggy. We have had to spend quite a bit of time every time we go back to review those bits to understand why we've done what we've done. Even as a temporary solution they're not very maintainable.

My proposal here is to have a version 2 which is not BC with previous releases in certain ways, and target a few breaking changes for it. We can aim to have the release in October 2023 or April/May 2024 as the version 2.

cc @scikit-learn/core-devs

COMMENT:
So it seems we don't have a disagreement on doing a breaking release for SLEP006, we might talk about other things to include later. I'll prepare a PR to simplify the code accordingly then.

Labels: RFC, Breaking Change
RFC: Breaking Changes for Version 2

A while ago we talked about the possibility of a version 2 with breaking changes. Specifically, this came up in the context of SLEP006: metadata routing, but there are other things we have wanted to break which we can do all in a single release. In this issue we can talk about the possibility of such a release, and maybe a few things we'd like to include in it.

I'll try to summarize the challenges of keeping backward compatibility (BC) in the context of SLEP006:

- `MetaEstimator(Estimator()).fit(X, y, sample_weight)` will raise according to SLEP006 since `sample_weight` is not requested by any object. For backward compatibility, during the transition period, it'll only warn and assume `sample_weight` is requested by `Estimator()`.
- What should the behavior of `GridSearchCV(Estimator(), scorer=scorer).fit(X, y, sample_weight)` be during the transition period? If we keep backward compatibility, it should warn (whatever the warning be), route `sample_weight` to `Estimator`, but not to the `scorer`. And that's only the case because we're keeping BC. From the user's perspective, it's very weird.
- `Pipeline` ATM in certain methods like `fit`, `fit_transform`, `predict`, routes `*_params` to the `fit/etc` of the last estimator only. And `decision_function` and `transform`, `score_samples` have no routing at all. Keeping this BC, proves to be challenging, and a bit nasty. ref: #24270
- In quite a few meta-estimators, we check if the sub-estimator has `sample_weight` in the signature of `fit`, and we pass the given sample weight to it if that's the case, and not if that's not the case. With routing, we would have a much better idea of when to pass sample weight and when not, but if we're keeping BC, it's really challenging to see if we should forward sample weights or not. Both AdaBoost (#24026) and Bagging (#24250) have been very challenging, to the point that we might end up having to check if a sub-estimator is a meta-estimator itself or not, and check signature of sub-estimator's `fit`, and check the given routing info to keep BC.

I'm pretty sure there are other issues we haven't foreseen, and they'll come up as we work on this project. We thought it'd be easy once we get to this point, but BC has proven to make it very tricky and has stalled the progress.

All of the above is adding substantial complexity which is probably error prone, and buggy. We have had to spend quite a bit of time every time we go back to review those bits to understand why we've done what we've done. Even as a temporary solution they're not very maintainable.

My proposal here is to have a version 2 which is not BC with previous releases in certain ways, and target a few breaking changes for it. We can aim to have the release in October 2023 or April/May 2024 as the version 2.

cc @scikit-learn/core-devs

COMMENT:
Do we need a SLEP for making a `BaseEstimator` / `scikit-learn-core` package?

Labels: RFC, Breaking Change
RFC: Breaking Changes for Version 2

A while ago we talked about the possibility of a version 2 with breaking changes. Specifically, this came up in the context of SLEP006: metadata routing, but there are other things we have wanted to break which we can do all in a single release. In this issue we can talk about the possibility of such a release, and maybe a few things we'd like to include in it.

I'll try to summarize the challenges of keeping backward compatibility (BC) in the context of SLEP006:

- `MetaEstimator(Estimator()).fit(X, y, sample_weight)` will raise according to SLEP006 since `sample_weight` is not requested by any object. For backward compatibility, during the transition period, it'll only warn and assume `sample_weight` is requested by `Estimator()`.
- What should the behavior of `GridSearchCV(Estimator(), scorer=scorer).fit(X, y, sample_weight)` be during the transition period? If we keep backward compatibility, it should warn (whatever the warning be), route `sample_weight` to `Estimator`, but not to the `scorer`. And that's only the case because we're keeping BC. From the user's perspective, it's very weird.
- `Pipeline` ATM in certain methods like `fit`, `fit_transform`, `predict`, routes `*_params` to the `fit/etc` of the last estimator only. And `decision_function` and `transform`, `score_samples` have no routing at all. Keeping this BC, proves to be challenging, and a bit nasty. ref: #24270
- In quite a few meta-estimators, we check if the sub-estimator has `sample_weight` in the signature of `fit`, and we pass the given sample weight to it if that's the case, and not if that's not the case. With routing, we would have a much better idea of when to pass sample weight and when not, but if we're keeping BC, it's really challenging to see if we should forward sample weights or not. Both AdaBoost (#24026) and Bagging (#24250) have been very challenging, to the point that we might end up having to check if a sub-estimator is a meta-estimator itself or not, and check signature of sub-estimator's `fit`, and check the given routing info to keep BC.

I'm pretty sure there are other issues we haven't foreseen, and they'll come up as we work on this project. We thought it'd be easy once we get to this point, but BC has proven to make it very tricky and has stalled the progress.

All of the above is adding substantial complexity which is probably error prone, and buggy. We have had to spend quite a bit of time every time we go back to review those bits to understand why we've done what we've done. Even as a temporary solution they're not very maintainable.

My proposal here is to have a version 2 which is not BC with previous releases in certain ways, and target a few breaking changes for it. We can aim to have the release in October 2023 or April/May 2024 as the version 2.

cc @scikit-learn/core-devs

COMMENT:
re: non-verbose declaration, we have this issue: https://github.com/scikit-learn/scikit-learn/issues/23928

Labels: RFC, Breaking Change
RFC: Breaking Changes for Version 2

A while ago we talked about the possibility of a version 2 with breaking changes. Specifically, this came up in the context of SLEP006: metadata routing, but there are other things we have wanted to break which we can do all in a single release. In this issue we can talk about the possibility of such a release, and maybe a few things we'd like to include in it.

I'll try to summarize the challenges of keeping backward compatibility (BC) in the context of SLEP006:

- `MetaEstimator(Estimator()).fit(X, y, sample_weight)` will raise according to SLEP006 since `sample_weight` is not requested by any object. For backward compatibility, during the transition period, it'll only warn and assume `sample_weight` is requested by `Estimator()`.
- What should the behavior of `GridSearchCV(Estimator(), scorer=scorer).fit(X, y, sample_weight)` be during the transition period? If we keep backward compatibility, it should warn (whatever the warning be), route `sample_weight` to `Estimator`, but not to the `scorer`. And that's only the case because we're keeping BC. From the user's perspective, it's very weird.
- `Pipeline` ATM in certain methods like `fit`, `fit_transform`, `predict`, routes `*_params` to the `fit/etc` of the last estimator only. And `decision_function` and `transform`, `score_samples` have no routing at all. Keeping this BC, proves to be challenging, and a bit nasty. ref: #24270
- In quite a few meta-estimators, we check if the sub-estimator has `sample_weight` in the signature of `fit`, and we pass the given sample weight to it if that's the case, and not if that's not the case. With routing, we would have a much better idea of when to pass sample weight and when not, but if we're keeping BC, it's really challenging to see if we should forward sample weights or not. Both AdaBoost (#24026) and Bagging (#24250) have been very challenging, to the point that we might end up having to check if a sub-estimator is a meta-estimator itself or not, and check signature of sub-estimator's `fit`, and check the given routing info to keep BC.

I'm pretty sure there are other issues we haven't foreseen, and they'll come up as we work on this project. We thought it'd be easy once we get to this point, but BC has proven to make it very tricky and has stalled the progress.

All of the above is adding substantial complexity which is probably error prone, and buggy. We have had to spend quite a bit of time every time we go back to review those bits to understand why we've done what we've done. Even as a temporary solution they're not very maintainable.

My proposal here is to have a version 2 which is not BC with previous releases in certain ways, and target a few breaking changes for it. We can aim to have the release in October 2023 or April/May 2024 as the version 2.

cc @scikit-learn/core-devs

COMMENT:
@adrinjalali Thanks for the good and precise summary 🙏

Labels: RFC, Breaking Change
RFC: Breaking Changes for Version 2

A while ago we talked about the possibility of a version 2 with breaking changes. Specifically, this came up in the context of SLEP006: metadata routing, but there are other things we have wanted to break which we can do all in a single release. In this issue we can talk about the possibility of such a release, and maybe a few things we'd like to include in it.

I'll try to summarize the challenges of keeping backward compatibility (BC) in the context of SLEP006:

- `MetaEstimator(Estimator()).fit(X, y, sample_weight)` will raise according to SLEP006 since `sample_weight` is not requested by any object. For backward compatibility, during the transition period, it'll only warn and assume `sample_weight` is requested by `Estimator()`.
- What should the behavior of `GridSearchCV(Estimator(), scorer=scorer).fit(X, y, sample_weight)` be during the transition period? If we keep backward compatibility, it should warn (whatever the warning be), route `sample_weight` to `Estimator`, but not to the `scorer`. And that's only the case because we're keeping BC. From the user's perspective, it's very weird.
- `Pipeline` ATM in certain methods like `fit`, `fit_transform`, `predict`, routes `*_params` to the `fit/etc` of the last estimator only. And `decision_function` and `transform`, `score_samples` have no routing at all. Keeping this BC, proves to be challenging, and a bit nasty. ref: #24270
- In quite a few meta-estimators, we check if the sub-estimator has `sample_weight` in the signature of `fit`, and we pass the given sample weight to it if that's the case, and not if that's not the case. With routing, we would have a much better idea of when to pass sample weight and when not, but if we're keeping BC, it's really challenging to see if we should forward sample weights or not. Both AdaBoost (#24026) and Bagging (#24250) have been very challenging, to the point that we might end up having to check if a sub-estimator is a meta-estimator itself or not, and check signature of sub-estimator's `fit`, and check the given routing info to keep BC.

I'm pretty sure there are other issues we haven't foreseen, and they'll come up as we work on this project. We thought it'd be easy once we get to this point, but BC has proven to make it very tricky and has stalled the progress.

All of the above is adding substantial complexity which is probably error prone, and buggy. We have had to spend quite a bit of time every time we go back to review those bits to understand why we've done what we've done. Even as a temporary solution they're not very maintainable.

My proposal here is to have a version 2 which is not BC with previous releases in certain ways, and target a few breaking changes for it. We can aim to have the release in October 2023 or April/May 2024 as the version 2.

cc @scikit-learn/core-devs

COMMENT:
> It's rather due to the inconsistencies and bugs we have in the codebase right now when it comes to metadata routing.

Just a stupid question: Would it make sense to first fix those bugs before introducing SLEP006?

Labels: RFC, Breaking Change