-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
[WIP] replaced n_iter by max_iter and added deprecation #7761
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
Conversation
sklearn/manifold/t_sne.py
Outdated
@@ -644,8 +645,10 @@ class TSNE(BaseEstimator): | |||
http://lvdmaaten.github.io/publications/papers/JMLR_2014.pdf | |||
""" | |||
|
|||
@property |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @property
is just for deprecating attributes, not functions. That's why the tests fail, I think.
You can isolate tests with the command: Note that this is currently breaking user's code, for example if they have |
Thanks for the feedback. I still get a few errors in the test coming from the https://travis-ci.org/scikit-learn/scikit-learn/jobs/171341699 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To pass the tests, you'll also have to add an attribute self.n_iter_
with the actual number of iteration done before convergence.
fit_intercept=True, normalize=False, copy_X=True, | ||
verbose=False): | ||
if n_iter is not None: | ||
warnings.warn("'n_iter' was deprecated. Use 'max_iter' instead.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please raise a DeprecationWarning
You also need to use this value if provided, so that the result of `ARDRegression(n_iter=10).fit(...)``is not modify by this PR:
self.n_iter = n_iter
if n_iter is not None:
warnings.warn(...)
self.max_iter = n_iter
else:
self.max_iter = max_iter
(same in other classes)
threshold_lambda=1.e+4, fit_intercept=True, normalize=False, | ||
copy_X=True, verbose=False): | ||
self.n_iter = n_iter | ||
@deprecated("Attribute n_iter was deprecated. Use 'max_iter' instead") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need to remove this line (same in other classes)
Reference Issue
#7736
What does this implement/fix? Explain your changes.
I have replaced
n_iter
bymax_iter
and added thedeprecated
decorators in the methods discussed in #7736.Any other comments?
The tests are not passing yet and I have a hard time to debug without test isolation. How can I isolate the file I want to test?