[MRG] Deprecated the rng_ attribute of GaussianProcessRegressor #8015
Conversation
| @property | ||
| @deprecated("Attribute ``rng_`` is deprecated in version 0.19 and will be" | ||
| " used only as a variable in version 0.21.") | ||
| def rng_(self): |
There was a problem hiding this comment.
there is no trailing underscore.
| @@ -140,6 +141,12 @@ def __init__(self, kernel=None, alpha=1e-10, | |||
| self.copy_X_train = copy_X_train | |||
| self.random_state = random_state | |||
|
|
|||
| @property | |||
| @deprecated("Attribute ``rng_`` is deprecated in version 0.19 and will be" | |||
| " used only as a variable in version 0.21.") | |||
There was a problem hiding this comment.
That's very unclear for the user. Say "will be removed"
There was a problem hiding this comment.
Added it from a developer point of view. But yes it would be unclear. Making the change.
|
LGTM once tests pass |
|
I know there is no standard way to do that in sklearn but it could be cool to add comment on how we clean the deprecation at 0.21. For example, in your case, you can add a comment saying : # Deprecation 0.21
# Replace the above line by
# rng = check_random_state(self.random_state)What do you think @amueller ? |
|
usually it's relatively obvious what is to do. I'm not against adding the comment but I don't think it's that helpful. In general, more documentation is better and this one doesn't really introduce any noise so I'd be good with it. |
|
@amueller Please let me know if I need to do anything else. |
I proposed that to simplify the future work but maybe it's not really necessary indeed :). @dalmia Sorry for the delay on your PR. |
|
@tguillemot Not a problem. :) |
|
I agree @jnothman, I'll close this then. |
Reference Issue
Fixes #7752
What does this implement/fix? Explain your changes.
Adds the deprecation for
rng_to be kept only as a variable forfit