Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Deprecate paired_distances and paired_*_distances #26982

Open
Micky774 opened this issue Aug 1, 2023 · 5 comments · May be fixed by #27129
Open

API Deprecate paired_distances and paired_*_distances #26982

Micky774 opened this issue Aug 1, 2023 · 5 comments · May be fixed by #27129
Labels
API help wanted Moderate Anything that requires some knowledge of conventions and best practices module:metrics

Comments

@Micky774
Copy link
Contributor

Micky774 commented Aug 1, 2023

Background

We have several public functions paired_*_distances which exist primarily as specialized implementations for the public paired_distances function. This function is thus either a wrapper around the specialized paired_*_distances or a simple iteration over the input data. During the drafting meeting, we discussed the deprecation of these functions. This discussion continued on a recent PR (start here). I wanted to open this to see if we could come to a quick consensus on our intent for the paired_* functions.

Personally, I definitely want to see paired_*_distances deprecated. I have no idea how widely used paired_distances is in practice. I believe @thomasjpfan has a better sense of this after some github searches. If it is not too widely used, I am also in favor of deprecating it as a whole.

As @adrinjalali mentioned (cf. comment) the paired_distances function is a really light function that most users ought to be able to write themselves. It doesn't really make sense for us to provide it for them...

cc: @scikit-learn/core-devs

Edit: Consensus is to deprecate both paired_*_distances and paired_distances

@Micky774 Micky774 changed the title API Deprecate paired_distances and paired_-_distances API Deprecate paired_distances and paired_*_distances Aug 1, 2023
@jjerphan
Copy link
Member

jjerphan commented Aug 4, 2023

I am +1 regarding deprecating and removing paired_*_distances.

I think we need to know if paired_distances is significantly used before taking any decision (I do not know any alternative to the global search of GitHub).

@adrinjalali
Copy link
Member

adrinjalali commented Aug 7, 2023

I'm in favor of deprecating them all, if too many users complain, we can reverse action, but the implementation is trivial, so we shouldn't have any issues removing them.

@Micky774
Copy link
Contributor Author

Micky774 commented Aug 8, 2023

@StefanieSenger this is a medium-scope task, in case you would be interested in taking it on. If not I can mark it as help wanted 😄

@Shreesha3112
Copy link
Contributor

Would like to open PR

@StefanieSenger
Copy link
Contributor

Seen the ping only now. If there is something I can support you with, @Shreesha3112, please let me know. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API help wanted Moderate Anything that requires some knowledge of conventions and best practices module:metrics
Projects
None yet
5 participants