GUDHI / gudhi-devel

The GUDHI library is a generic open source C++ library, with a Python interface, for Topological Data Analysis (TDA) and Higher Dimensional Geometry Understanding.
https://gudhi.inria.fr/
MIT License
246 stars 65 forks source link

fixed deprecated argument #990

Closed MathieuCarriere closed 8 months ago

MathieuCarriere commented 8 months ago

Fix for #882

mglisse commented 8 months ago

Just checking: do we have to mention somewhere that gudhi requires version at least x.y of this dependency? (I assume metric was not available in the past)

MathieuCarriere commented 8 months ago

Yes, using argument metric won't work for Scikit-Learn < 1.2, which only knows affinity. So the code of this PR only works for Scikit-Learn version 1.2 or more.

VincentRouvreau commented 8 months ago

Yes, using argument metric won't work for Scikit-Learn < 1.2, which only knows affinity. So the code of this PR only works for Scikit-Learn version 1.2 or more.

I confirm : https://scikit-learn.org/stable/whats_new/v1.2.html#id12 As scikit-learn 1.2.0 is quite recent (december 2022), I won't ask for scikit-learn >= 1.2.0 as a dependency. We could do something like:

from sklearn import __version__ as sklearn_version
from pkg_resources import packaging

agglomerative_clustering_metric = packaging.version.parse(sklearn_version) >= packaging.version.parse('1.2.0')

...
            if self.clustering is None:
                if self.input_type == "point cloud":
                    kwarg =  {'metric':'euclidean'} if agglomerative_clustering_metric else {'affinity':'euclidean'}
                    self.clustering = AgglomerativeClustering(n_clusters=None, linkage="single", distance_threshold=delta, **kwarg)
                else:
                    kwarg =  {'metric':'precomputed'} if agglomerative_clustering_metric else {'affinity':'precomputed'}
                    self.clustering = AgglomerativeClustering(n_clusters=None, linkage="single", distance_threshold=delta, **kwarg)
MathieuCarriere commented 8 months ago

Done!