-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
[R-package] remove support for '...' in predict() #4857
Conversation
R-package/R/lgb.Booster.R
Outdated
@@ -828,23 +815,12 @@ predict.lgb.Booster <- function(object, | |||
predcontrib = FALSE, | |||
header = FALSE, | |||
reshape = FALSE, | |||
params = list(), | |||
...) { | |||
params = list()) { |
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.
It looks like ...
cannot be removed from predict.lgb.Booster()
, since the predict()
generic in base R has a ...
.
From R CMD check
(build link)
* checking S3 generic/method consistency ... WARNING
predict:
function(object, ...)
predict.lgb.Booster:
function(object, data, start_iteration, num_iteration, rawscore,
predleaf, predcontrib, header, reshape, params)
See section ‘Generic functions and methods’ in the ‘Writing R
Extensions’ manual.
From https://cran.r-project.org/doc/manuals/R-exts.html#Generic-functions-and-methods
A method must have all the arguments of the generic, including … if the generic does.
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.
Add a warning and updated docs in 49c8368, so that at least users are notified that ...
is not intended for use with any {lightgbm}
parameters.
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.
LGTM! Thanks for the explanation of generic predict()
function.
This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this. |
Contributes to #4226.
This PR removes support for passing anything through
...
inpredict.lgb.Booster()
andBooster$predict()
.Notes for Reviewers
v3.3.0 and v3.3.1 contain a deprecation warning about this change.