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

DEP: stats.kendalltau: remove deprecated parameter initial_lexsort #18593

Closed
wants to merge 1 commit into from

Conversation

mdhaber
Copy link
Contributor

@mdhaber mdhaber commented May 31, 2023

Reference issue

Closes gh-18578

What does this implement/fix?

This removes deprecated parameter initial_lexsort.

Additional information

Marked as draft pending resolution of #18578 (comment).

@mdhaber mdhaber added scipy.stats deprecated Items related to behavior that has been deprecated labels May 31, 2023
@mdhaber mdhaber marked this pull request as draft May 31, 2023 04:38
@rgommers
Copy link
Member

To follow up on #18578 (comment), it looks to me like this keyword can't be removed at ll, because the deprecation was incomplete. kendalltau(x, y, None, 'omit', 'exact') never generated a warning, so the deprecation itself should be adjusted to do that now. And then in two releases from now, it can indeed be made keyword-only.

@j-bowhay
Copy link
Member

j-bowhay commented May 31, 2023

To follow up on #18578 (comment), it looks to me like this keyword can't be removed at ll, because the deprecation was incomplete. kendalltau(x, y, None, 'omit', 'exact') never generated a warning, so the deprecation itself should be adjusted to do that now. And then in two releases from now, it can indeed be made keyword-only.

This is an interesting point that doesn't seem to have been brought up before. We have assumed this sort of thing was alright for a long time eg. #6948 without any complaints. Is this something we want to clean up? If so there are quite a lot of deprecations that are going to need bumping.

@rgommers
Copy link
Member

We have assumed this sort of thing was alright for a long time eg. #6948 without any complaints.

In that example it was changed in the PR from False to None, presumably exactly to emit a warning if a user indeed used the previous default (False) explicitly. So the consideration isn't new I'd say. It was also a pretty niche keyword.

I think if it gets too non-idiomatic or weird, then we don't have to insist on a deprecation warning being 100% foolproof. But in this case it seems like a regular way to use positional arguments.

@mdhaber mdhaber closed this May 31, 2023
@j-bowhay
Copy link
Member

@h-vetinari quite a lot of the issues are going to need adjusting based on this concern

@h-vetinari
Copy link
Member

@h-vetinari quite a lot of the issues are going to need adjusting based on this concern

"this concern" being any changes to arguments that can also be used positionally? FWIW, we've been removing deprecated keyword(-also, not -only) arguments for the last few releases already (see log), but I guess it was only a handful each time and so the blast radius was limited.

@j-bowhay
Copy link
Member

@h-vetinari quite a lot of the issues are going to need adjusting based on this concern

"this concern" being any changes to arguments that can also be used positionally? FWIW, we've been removing deprecated keyword(-also, not -only) arguments for the last few releases already (see log), but I guess it was only a handful each time and so the blast radius was limited.

Exactly. Unless you object, I am planning on going through all the current deprecations over the next couple of weeks to implement @rgommers suggestion where applicable. I imagine this is going to involve pushing the timeline back on a number of them. I am also planning on updating the dev docs section on deprecations to reflect the best practices we have accumulated over the last year or so

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecated Items related to behavior that has been deprecated scipy.stats
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DEP: deprecate positional arguments in stats.kendalltau
4 participants