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

Shreesha3112 · 2023-08-22T09:50:46Z

Reference Issues/PRs

Fixes #26982

What does this implement/fix? Explain your changes.

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

metrics.pairwise.paired_euclidean_distances
metrics.pairwise.paired_manhattan_distances
metrics.pairwise.paired_cosine_distances
metrics.pairwise.paired_distances

Any other comments?

…distance_public_funcs

github-actions · 2023-08-22T09:52:39Z

✔️ Linting Passed

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

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

…distance_public_funcs

Shreesha3112 · 2023-08-22T10:24:20Z

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?

ogrisel · 2023-08-22T12:18:38Z

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.

…distance_public_funcs

glemaitre · 2023-10-31T16:03:31Z

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

glemaitre

I am fine with the deprecation. I only commented to add some info that we use when we grep the files seeking for deprecated function/variables.

sklearn/metrics/pairwise.py

sklearn/metrics/tests/test_pairwise.py

glemaitre · 2023-10-31T16:18:39Z

sklearn/tests/test_public_functions.py

@@ -275,10 +275,6 @@ def _check_function_param_validation(
    "sklearn.metrics.pairwise.linear_kernel",
    "sklearn.metrics.pairwise.manhattan_distances",
    "sklearn.metrics.pairwise.nan_euclidean_distances",
-    "sklearn.metrics.pairwise.paired_cosine_distances",


I would still test them but add a comment above to remove them.

I think this still needs to be reverted.

glemaitre

I am fine with the deprecation. I only commented to add some info that we use when we grep the files seeking for deprecated function/variables. Thanks @Shreesha3112

glemaitre

I am fine with the deprecation. I only commented to add some info that we use when we grep the files seeking for deprecated function/variables. Thanks @Shreesha3112

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>

amueller

Looks good but I'd add a note to the deprecation message that tells users what function to use instead (pairwise_distances I assume).

thomasjpfan

Looks good but I'd add a note to the deprecation message that tells users what function to use instead (pairwise_distances I assume).

@amueller If the paired_*_distances are removed, there is no direct replacement.

thomasjpfan · 2024-02-23T00:06:08Z

doc/modules/classes.rst

-   metrics.pairwise.paired_euclidean_distances
-   metrics.pairwise.paired_manhattan_distances
-   metrics.pairwise.paired_cosine_distances
-   metrics.pairwise.paired_distances


Can these be moved to

scikit-learn/doc/modules/classes.rst

Lines 1903 to 1904 in 93f6ce9

Recently deprecated

===================

?

thomasjpfan · 2024-02-23T00:06:40Z

doc/whats_new/v1.4.rst

+  :func:`~metrics.pairwise.paired_euclidean_distances`,
+  :func:`~metrics.pairwise.paired_manhattan_distances` and
+  :func:`~metrics.pairwise.paired_cosine_distances` are now deprecated and
+  will be removed in 1.6.


Since 1.4 is released, this changelog entry needs to be moved to v1.5.rst and:

Suggested change

will be removed in 1.6.

will be removed in 1.7.

thomasjpfan · 2024-02-23T00:06:49Z

sklearn/metrics/pairwise.py

@@ -1129,6 +1130,11 @@ def cosine_distances(X, Y=None):
    return S


+# TODO(1.6): Remove in 1.6


Suggested change

# TODO(1.6): Remove in 1.6

# TODO(1.7): Remove in 1.7

thomasjpfan · 2024-02-23T00:06:57Z

sklearn/metrics/pairwise.py

+# TODO(1.6): Remove in 1.6
+@deprecated(
+    "The public function `sklearn.pairwise.paired_euclidean_distances` has been "
+    "deprecated in 1.4 and will be removed in 1.6."


Suggested change

"deprecated in 1.4 and will be removed in 1.6."

"deprecated in 1.5 and will be removed in 1.7."

thomasjpfan · 2024-02-23T00:07:14Z

sklearn/metrics/pairwise.py

@@ -1157,6 +1186,11 @@ def paired_euclidean_distances(X, Y):
    return row_norms(X - Y)


+# TODO(1.6): Remove in 1.6


Suggested change

# TODO(1.6): Remove in 1.6

# TODO(1.7): Remove in 1.7

thomasjpfan · 2024-02-23T00:07:30Z

sklearn/metrics/pairwise.py

+# TODO*1.6: Remove in 1.6
+@deprecated(
+    "The public function `sklearn.pairwise.paired_cosine_distances` has been "
+    "deprecated in 1.4 and will be removed in 1.6."


Suggested change

"deprecated in 1.4 and will be removed in 1.6."

"deprecated in 1.5 and will be removed in 1.7."

thomasjpfan · 2024-02-23T00:07:38Z

sklearn/metrics/pairwise.py

@@ -1201,6 +1260,11 @@ def paired_manhattan_distances(X, Y):
        return np.abs(diff).sum(axis=-1)


+# TODO*1.6: Remove in 1.6


Suggested change

# TODO*1.6: Remove in 1.6

# TODO(1.7): Remove in 1.7

thomasjpfan · 2024-02-23T00:07:45Z

sklearn/metrics/pairwise.py


+# TODO(1.6): Remove in 1.6


Suggested change

# TODO(1.6): Remove in 1.6

# TODO(1.7): Remove in 1.7

thomasjpfan · 2024-02-23T00:07:52Z

sklearn/metrics/pairwise.py

+# TODO(1.6): Remove in 1.6
+@deprecated(
+    "The public function `sklearn.pairwise.paired_distances` has been "
+    "deprecated in 1.4 and will be removed in 1.6."


Suggested change

"deprecated in 1.4 and will be removed in 1.6."

"deprecated in 1.5 and will be removed in 1.7."

thomasjpfan · 2024-02-23T00:08:39Z

sklearn/tests/test_public_functions.py

@@ -275,10 +275,6 @@ def _check_function_param_validation(
    "sklearn.metrics.pairwise.linear_kernel",
    "sklearn.metrics.pairwise.manhattan_distances",
    "sklearn.metrics.pairwise.nan_euclidean_distances",
-    "sklearn.metrics.pairwise.paired_cosine_distances",


I think this still needs to be reverted.

Micky774 · 2024-09-26T18:47:31Z

@Shreesha3112 since the core work is done, and all that remains is some small tidying, I wanted to remind you of this in case you'd still like to finish!

Sean-Jay-M · 2024-10-29T17:41:35Z

@Shreesha3112 since the core work is done, and all that remains is some small tidying, I wanted to remind you of this in case you'd still like to finish!

@Micky774, @thomasjpfan Is this considered stalled ?

I'll take a look at cleaning this up and finishing it since it appears @Shreesha3112 has gone dormant. Will start in the coming days if we get no response.

SuccessMoses · 2024-12-22T21:55:39Z

@Sean-Jay-M are you working on this?

Sean-Jay-M · 2024-12-23T00:43:04Z

@Sean-Jay-M are you working on this?

Hey I'm not currently working on this. Trying to close out a different PR. Feel free to take the issue. I never got a response from the maintainers if it should be considered a stalled PR. I think should be considered stalled.

Micky774 · 2024-12-23T19:07:27Z

Yes at this point the PR is stalled and continuation of the effort is welcomed!

shreesha3112 added 2 commits August 22, 2023 12:42

deprecate metrics.pairwise.paired_*_distances public functions

6cb4c44

Merge remote-tracking branch 'upstream/main' into deprecate_pairwise_…

1f91db3

…distance_public_funcs

github-actions bot added the module:metrics label Aug 22, 2023

Shreesha3112 changed the title ~~API Deprecate metrics.pairwise.paired_*_distance public functions~~ MNT Deprecate metrics.pairwise.paired_*_distance public functions Aug 22, 2023

shreesha3112 added 2 commits August 22, 2023 15:34

update PR number in changelog

270ec8e

Merge remote-tracking branch 'upstream/main' into deprecate_pairwise_…

5b84071

…distance_public_funcs

Shreesha3112 marked this pull request as ready for review August 22, 2023 10:25

shreesha3112 added 3 commits August 24, 2023 17:21

added private paired_functons and updated agglometrative clustering

aff52bf

resolve merge conflict

d7821b3

Removed paired_distance from public classes

bac00ac

Shreesha3112 changed the title ~~MNT Deprecate metrics.pairwise.paired_*_distance public functions~~ MNT Deprecate metrics.pairwise.paired_*_distances and paired_distances public functions Aug 24, 2023

shreesha3112 added 4 commits August 24, 2023 20:57

add deprecation for paired_distances

58f92b6

Merge remote-tracking branch 'upstream/main' into deprecate_pairwise_…

74fb1f6

…distance_public_funcs

remove paired_distance public functions from param_validation_list

708b696

Merge remote-tracking branch 'upstream/main' into deprecate_pairwise_…

0e2629c

…distance_public_funcs

glemaitre self-requested a review October 31, 2023 15:59

Merge remote-tracking branch 'origin/main' into pr/Shreesha3112/27129

928c6f4

glemaitre approved these changes Oct 31, 2023

View reviewed changes

glemaitre added the Waiting for Second Reviewer First reviewer is done, need a second one! label Oct 31, 2023

Apply suggestions from code review

aaf36e6

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>

amueller approved these changes Feb 9, 2024

View reviewed changes

thomasjpfan reviewed Feb 23, 2024

View reviewed changes

Micky774 added the Stalled label Dec 23, 2024

SuccessMoses mentioned this pull request Dec 24, 2024

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

Open

virchan mentioned this pull request Feb 5, 2025

Add Weighted Euclidean Distance Metric #30732

Open

adrinjalali removed the Waiting for Second Reviewer First reviewer is done, need a second one! label Apr 30, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Shreesha3112 commented Aug 22, 2023 •

edited

Loading

github-actions bot commented Aug 22, 2023 •

edited

Loading

Shreesha3112 commented Aug 22, 2023

ogrisel commented Aug 22, 2023

glemaitre commented Oct 31, 2023

glemaitre left a comment

glemaitre Oct 31, 2023

thomasjpfan Feb 23, 2024

glemaitre left a comment

glemaitre left a comment

amueller left a comment

thomasjpfan left a comment

thomasjpfan Feb 23, 2024

thomasjpfan Feb 23, 2024

thomasjpfan Feb 23, 2024

thomasjpfan Feb 23, 2024

thomasjpfan Feb 23, 2024

thomasjpfan Feb 23, 2024

thomasjpfan Feb 23, 2024

thomasjpfan Feb 23, 2024

thomasjpfan Feb 23, 2024

thomasjpfan Feb 23, 2024

Micky774 commented Sep 26, 2024

Sean-Jay-M commented Oct 29, 2024 •

edited

Loading

SuccessMoses commented Dec 22, 2024

Sean-Jay-M commented Dec 23, 2024 •

edited

Loading

Micky774 commented Dec 23, 2024

		@@ -1129,6 +1130,11 @@ def cosine_distances(X, Y=None):
		return S


		# TODO(1.6): Remove in 1.6

	"deprecated in 1.4 and will be removed in 1.6."
	"deprecated in 1.5 and will be removed in 1.7."

		@@ -1157,6 +1186,11 @@ def paired_euclidean_distances(X, Y):
		return row_norms(X - Y)


		# TODO(1.6): Remove in 1.6

		@@ -1201,6 +1260,11 @@ def paired_manhattan_distances(X, Y):
		return np.abs(diff).sum(axis=-1)


		# TODO*1.6: Remove in 1.6

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

Are you sure you want to change the base?

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

Conversation

Shreesha3112 commented Aug 22, 2023 • edited Loading

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

github-actions bot commented Aug 22, 2023 • edited Loading

✔️ Linting Passed

Shreesha3112 commented Aug 22, 2023

ogrisel commented Aug 22, 2023

glemaitre commented Oct 31, 2023

glemaitre left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glemaitre left a comment

Choose a reason for hiding this comment

glemaitre left a comment

Choose a reason for hiding this comment

amueller left a comment

Choose a reason for hiding this comment

thomasjpfan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Micky774 commented Sep 26, 2024

Sean-Jay-M commented Oct 29, 2024 • edited Loading

SuccessMoses commented Dec 22, 2024

Sean-Jay-M commented Dec 23, 2024 • edited Loading

Micky774 commented Dec 23, 2024

Shreesha3112 commented Aug 22, 2023 •

edited

Loading

github-actions bot commented Aug 22, 2023 •

edited

Loading

Sean-Jay-M commented Oct 29, 2024 •

edited

Loading

Sean-Jay-M commented Dec 23, 2024 •

edited

Loading