Repository: scikit-learn/scikit-learn
Pull request: MNT Deprecate `affinity` in `AgglomerativeClustering`
#### Reference Issues/PRs
Resolves #13945

#### What does this implement/fix? Explain your changes.
Deprecates `affinity` in favor of `metric` in `AgglomerativeClustering`

#### Any other comments?
Probably worth propagating the deprecation to `linkage_tree` in a follow-up PR.

Problem statement:
[AgglomerativeClustering] confusing parameter 'affinity'
The name of the parameter 'affinity' is misleading, since it does not correspond to similarity/affinity functions, but to distance functions.

Besides, the documentation for the parameter 'affinity' seems wrong.

> Metric used to compute the linkage. Can be “euclidean”, “l1”, “l2”, “manhattan”, “cosine”, or “precomputed”. If linkage is “ward”, only “euclidean” is accepted. If “precomputed”, a distance matrix (instead of a similarity matrix) is needed as input for the fit method.

If `affinity="precomputed"` a distance matrix, **instead of a feature matrix (not a similarity matrix)**, is needed as input for the fit method ?

Hints from issue discussion:
Yes, I think we've been aware of this for a long time. I suspect you're
right that we should "bite the bullet" and rename it to `metric`, with
deprecation of course.

Commit message: Addressed reviewer feedback

Patch to review:
diff --git a/doc/whats_new/v1.2.rst b/doc/whats_new/v1.2.rst
index ede154619526e..5e43e7fea8b92 100644
--- a/doc/whats_new/v1.2.rst
+++ b/doc/whats_new/v1.2.rst
@@ -85,6 +85,10 @@ Changelog
   `eigen_tol="auto"` in version 1.3.
   :pr:`23210` by :user:`Meekail Zain <micky774>`.
 
+- |API| The `affinity` attribute is now deprecated for
+  :class:`cluster.AgglomerativeClustering` and will be renamed to `metric` in v1.4.
+  :pr:`23470` by :user:`Meekail Zain <micky774>`.
+
 :mod:`sklearn.datasets`
 .......................
 
diff --git a/sklearn/cluster/_agglomerative.py b/sklearn/cluster/_agglomerative.py
index b399f805a9d40..c527c6ce37e5b 100644
--- a/sklearn/cluster/_agglomerative.py
+++ b/sklearn/cluster/_agglomerative.py
@@ -755,6 +755,19 @@ class AgglomerativeClustering(ClusterMixin, BaseEstimator):
         If "precomputed", a distance matrix (instead of a similarity matrix)
         is needed as input for the fit method.
 
+        .. deprecated:: 1.2
+            `affinity` was deprecated in version 1.2 and will be renamed to
+            `metric` in 1.4.
+
+    metric : str or callable, default='euclidean'
+        Metric used to compute the linkage. Can be "euclidean", "l1", "l2",
+        "manhattan", "cosine", or "precomputed".
+        If linkage is "ward", only "euclidean" is accepted.
+        If "precomputed", a distance matrix (instead of a similarity matrix)
+        is needed as input for the fit method.
+
+        .. versionadded:: 1.2
+
     memory : str or object with the joblib.Memory interface, default=None
         Used to cache the output of the computation of the tree.
         By default, no caching is done. If a string is given, it is the
@@ -876,7 +889,8 @@ def __init__(
         self,
         n_clusters=2,
         *,
-        affinity="euclidean",
+        affinity="deprecated",  # TODO(1.4): Remove
+        metric="euclidean",
         memory=None,
         connectivity=None,
         compute_full_tree="auto",
@@ -891,6 +905,7 @@ def __init__(
         self.compute_full_tree = compute_full_tree
         self.linkage = linkage
         self.affinity = affinity
+        self.metric = metric
         self.compute_distances = compute_distances
 
     def fit(self, X, y=None):
@@ -901,7 +916,7 @@ def fit(self, X, y=None):
         X : array-like, shape (n_samples, n_features) or \
                 (n_samples, n_samples)
             Training instances to cluster, or distances between instances if
-            ``affinity='precomputed'``.
+            ``metric='precomputed'``.
 
         y : Ignored
             Not used, present here for API consistency by convention.
@@ -930,6 +945,16 @@ def _fit(self, X):
         """
         memory = check_memory(self.memory)
 
+        self._metric = self.metric
+        # TODO(1.4): Remove
+        if self.affinity != "deprecated":
+            warnings.warn(
+                "Attribute `affinity` was deprecated in version 1.2 and will be removed"
+                " in 1.4. Use `metric` instead",
+                DeprecationWarning,
+            )
+            self._metric = self.affinity
+
         if self.n_clusters is not None and self.n_clusters <= 0:
             raise ValueError(
                 "n_clusters should be an integer greater than 0. %s was provided."
@@ -948,10 +973,10 @@ def _fit(self, X):
                 "compute_full_tree must be True if distance_threshold is set."
             )
 
-        if self.linkage == "ward" and self.affinity != "euclidean":
+        if self.linkage == "ward" and self._metric != "euclidean":
             raise ValueError(
-                "%s was provided as affinity. Ward can only "
-                "work with euclidean distances." % (self.affinity,)
+                "%s was provided as metric. Ward can only "
+                "work with euclidean distances." % (self._metric,)
             )
 
         if self.linkage not in _TREE_BUILDERS:
@@ -989,7 +1014,7 @@ def _fit(self, X):
         kwargs = {}
         if self.linkage != "ward":
             kwargs["linkage"] = self.linkage
-            kwargs["affinity"] = self.affinity
+            kwargs["affinity"] = self._metric
 
         distance_threshold = self.distance_threshold
 
@@ -1071,6 +1096,17 @@ class FeatureAgglomeration(
         "manhattan", "cosine", or 'precomputed'.
         If linkage is "ward", only "euclidean" is accepted.
 
+        .. deprecated:: 1.2
+            `affinity` was deprecated in version 1.2 and will be renamed to
+            `metric` in 1.4.
+
+    metric : str or callable, default='euclidean'
+        Metric used to compute the linkage. Can be "euclidean", "l1", "l2",
+        "manhattan", "cosine", or 'precomputed'.
+        If linkage is "ward", only "euclidean" is accepted.
+
+        .. versionadded:: 1.2
+
     memory : str or object with the joblib.Memory interface, default=None
         Used to cache the output of the computation of the tree.
         By default, no caching is done. If a string is given, it is the
@@ -1196,7 +1232,8 @@ def __init__(
         self,
         n_clusters=2,
         *,
-        affinity="euclidean",
+        affinity="deprecated",  # TODO(1.4): Remove
+        metric="euclidean",
         memory=None,
         connectivity=None,
         compute_full_tree="auto",
@@ -1212,6 +1249,7 @@ def __init__(
             compute_full_tree=compute_full_tree,
             linkage=linkage,
             affinity=affinity,
+            metric=metric,
             distance_threshold=distance_threshold,
             compute_distances=compute_distances,
         )
diff --git a/sklearn/cluster/tests/test_hierarchical.py b/sklearn/cluster/tests/test_hierarchical.py
index c6607779f80fc..b49d1adc9a4b5 100644
--- a/sklearn/cluster/tests/test_hierarchical.py
+++ b/sklearn/cluster/tests/test_hierarchical.py
@@ -8,6 +8,7 @@
 import itertools
 from tempfile import mkdtemp
 import shutil
+import warnings
 import pytest
 from functools import partial
 
@@ -243,7 +244,7 @@ def test_agglomerative_clustering():
     clustering = AgglomerativeClustering(
         n_clusters=10,
         connectivity=connectivity.toarray(),
-        affinity="manhattan",
+        metric="manhattan",
         linkage="ward",
     )
     with pytest.raises(ValueError):
@@ -255,12 +256,12 @@ def test_agglomerative_clustering():
         clustering = AgglomerativeClustering(
             n_clusters=10,
             connectivity=np.ones((n_samples, n_samples)),
-            affinity=affinity,
+            metric=affinity,
             linkage="complete",
         )
         clustering.fit(X)
         clustering2 = AgglomerativeClustering(
-            n_clusters=10, connectivity=None, affinity=affinity, linkage="complete"
+            n_clusters=10, connectivity=None, metric=affinity, linkage="complete"
         )
         clustering2.fit(X)
         assert_almost_equal(
@@ -277,7 +278,7 @@ def test_agglomerative_clustering():
     clustering2 = AgglomerativeClustering(
         n_clusters=10,
         connectivity=connectivity,
-        affinity="precomputed",
+        metric="precomputed",
         linkage="complete",
     )
     clustering2.fit(X_dist)
@@ -291,7 +292,7 @@ def test_agglomerative_clustering_memory_mapped():
     """
     rng = np.random.RandomState(0)
     Xmm = create_memmap_backed_data(rng.randn(50, 100))
-    AgglomerativeClustering(affinity="euclidean", linkage="single").fit(Xmm)
+    AgglomerativeClustering(metric="euclidean", linkage="single").fit(Xmm)
 
 
 def test_ward_agglomeration():
@@ -876,7 +877,7 @@ def test_invalid_shape_precomputed_dist_matrix():
         ValueError,
         match=r"Distance matrix should be square, got matrix of shape \(5, 3\)",
     ):
-        AgglomerativeClustering(affinity="precomputed", linkage="complete").fit(X)
+        AgglomerativeClustering(metric="precomputed", linkage="complete").fit(X)
 
 
 def test_precomputed_connectivity_affinity_with_2_connected_components():
@@ -916,3 +917,27 @@ def test_precomputed_connectivity_affinity_with_2_connected_components():
 
     assert_array_equal(clusterer.labels_, clusterer_precomputed.labels_)
     assert_array_equal(clusterer.children_, clusterer_precomputed.children_)
+
+
+# TODO(1.4): Remove
+def test_deprecate_affinity():
+    rng = np.random.RandomState(42)
+    X = rng.randn(50, 10)
+    est = [
+        AgglomerativeClustering(),
+        AgglomerativeClustering(metric="euclidean"),
+    ]
+    with warnings.catch_warnings():
+        warnings.simplefilter("error", DeprecationWarning)
+        for af in est:
+            af.fit(X)
+            af.fit_predict(X)
+
+    af = AgglomerativeClustering(affinity="euclidean")
+    msg = (
+        "Attribute `affinity` was deprecated in version 1.2 and will be removed in 1.4."
+        " Use `metric` instead"
+    )
+    with pytest.warns(DeprecationWarning, match=msg):
+        af.fit(X)
+        af.fit_predict(X)


Code review comments:
diff_hunk: @@ -930,6 +945,16 @@ def _fit(self, X):
         """
         memory = check_memory(self.memory)
 
+        self._metric = self.metric
+        # TODO(1.4): Remove
+        if self.affinity != "deprecated":
+            warnings.warn(
+                "Attribute `affinity` was deprecated in version 1.2 and will be removed"
+                " in 1.4. Use `metric` instead",
+                DeprecationWarning,
path: sklearn/cluster/_agglomerative.py
text: @user1:
The reason we use `FutureWarning` is that is appears by default when the function is imported from another file.

```suggestion
                FutureWarning,
```