Skip to content

MNT change 0.25 to 1.0 and 0.26 to 1.1 in deprecation messages #19005

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

Merged

Conversation

glemaitre
Copy link
Member

Change the version in FutureWarning after making the choice of releasing a major upcoming release (1.0)

@NicolasHug
Copy link
Member

Should we write "1.0 (renaming of 0.25)" and "1.1 (renaming of 0.26)" for the continuity to be clear?

Usually, messages are the form "deprecated in version x and will be removed in version x+2" and that "+2" part is lost now, so unless people are aware that 1.0 == 0.25, they'll have no idea when the deprecation will be final.

@glemaitre
Copy link
Member Author

We can do that for 0.25 since we already issued the warning in the previous release. For the 0.26, we never issued the warning do you think that this is indeed necessary?

@ogrisel
Copy link
Member

ogrisel commented Dec 16, 2020

Should we write "1.0 (renaming of 0.25)" and "1.1 (renaming of 0.26)" for the continuity to be clear?

I like this suggestion.

@NicolasHug
Copy link
Member

Yes I think it's still relevant for 0.26/1.1 because "this is deprecated in 0.24 and will be removed in 1.1" says nothing about what 1.1 is w.r.t. 0.24, or when 1.1 will be released

@ogrisel ogrisel added the To backport PR merged in master that need a backport to a release branch defined based on the milestone. label Dec 16, 2020
@ogrisel ogrisel added this to the 0.24 milestone Dec 16, 2020
Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @glemaitre , LGTM

Maybe this is something we can merge with just 1 approval if we need to move things forward, so I'd say feel free to self-merge

@glemaitre
Copy link
Member Author

Maybe this is something we can merge with just 1 approval if we need to move things forward, so I'd say feel free to self-merge

I would not be against this. This is mechanical procedure.

Comment on lines 231 to 232
The _pairwise attribute is deprecated in 0.24. From 1.1 onward
(renaming of 0.26), the `pairwise` estimator tag should be used instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The _pairwise attribute is deprecated in 0.24. From 1.1 onward
(renaming of 0.26), the `pairwise` estimator tag should be used instead.
The _pairwise attribute is deprecated in 0.24. From 1.1 (renaming of 0.26)
onward, the `pairwise` estimator tag should be used instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I a bit confused, is it the opposit of what Nicolas wanted or that you proposed the same change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's NBD, merging ;)

@NicolasHug NicolasHug changed the title MNT change version in FutureWarning MNT change 0.25 to 1.0 and 0.26 to 1.1 in deprecation messages Dec 18, 2020
@NicolasHug NicolasHug merged commit 2218ec4 into scikit-learn:master Dec 18, 2020
@glemaitre
Copy link
Member Author

glemaitre commented Dec 18, 2020 via email

glemaitre added a commit to glemaitre/scikit-learn that referenced this pull request Dec 22, 2020
@glemaitre glemaitre mentioned this pull request Dec 22, 2020
14 tasks
@glemaitre glemaitre mentioned this pull request Apr 22, 2021
12 tasks
glemaitre added a commit to glemaitre/scikit-learn that referenced this pull request Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
To backport PR merged in master that need a backport to a release branch defined based on the milestone.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants