scikit-learn icon indicating copy to clipboard operation
scikit-learn copied to clipboard

MNT Deprecate metrics.pairwise.paired_*_distances and paired_distances public functions

Open Shreesha3112 opened this issue 2 years ago • 5 comments

Reference Issues/PRs

Fixes #26982

What does this implement/fix? Explain your changes.

Deprecates metrics.pairwise.paired_*_distance functions and metrics.pairwise.paired_distance

  • [x] metrics.pairwise.paired_euclidean_distances
  • [x] metrics.pairwise.paired_manhattan_distances
  • [x] metrics.pairwise.paired_cosine_distances
  • [x] metrics.pairwise.paired_distances

Any other comments?

Shreesha3112 avatar Aug 22 '23 09:08 Shreesha3112

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: aaf36e6. Link to the linter CI: here

github-actions[bot] avatar Aug 22 '23 09:08 github-actions[bot]

I've noticed that the changes to metrics.pairwise.paired_distance could impact cluster.AgglomerativeClustering. Specifically, the linkage_tree function relies on metrics.pairwise.paired_distances when the affinity isn't precomputed. Given this, should I go ahead and make the required changes to both metrics.pairwise.paired_distances and the linkage_tree function within this PR, or would it be preferred to handle them separately?

Shreesha3112 avatar Aug 22 '23 10:08 Shreesha3112

Maybe we could put the code in private functions (with a _ prefix and without redundant input parameter and data checks) and make the deprecated public functions call their private counterparts.

The AgglomerativeClustering estimator could then directly call the private functions.

ogrisel avatar Aug 22 '23 12:08 ogrisel

I resolved the conflicts with main. I will give a round of review.

glemaitre avatar Oct 31 '23 16:10 glemaitre