-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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] Working on Roxygen documentation #10674
Changes from 3 commits
f5249ad
3b69935
5e57bfe
c6c2012
4fb238b
9a5314f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,10 +10,8 @@ S3method(getinfo,xgb.Booster) | |
S3method(getinfo,xgb.DMatrix) | ||
S3method(length,xgb.Booster) | ||
S3method(predict,xgb.Booster) | ||
S3method(print,xgb.Booster) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this will break existing behaviors There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is part of the work on the new R interface: #9810 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It shouldn't be related to the changes in other PRs. In the current master branch, triggering docs build with roxygen doesn't remove the S3 method exports. Removing the S3 exports would indeed break things elsewhere. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
S3method(print,xgb.DMatrix) | ||
S3method(print,xgb.cv.synchronous) | ||
S3method(print,xgboost) | ||
S3method(setinfo,xgb.Booster) | ||
S3method(setinfo,xgb.DMatrix) | ||
S3method(variable.names,xgb.Booster) | ||
|
@@ -22,6 +20,8 @@ export("xgb.attributes<-") | |
export("xgb.config<-") | ||
export("xgb.parameters<-") | ||
export(getinfo) | ||
export(print.xgb.Booster) | ||
export(print.xgboost) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did we remove any explicit s3 export statements? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have added a commit that fixes the problem by adding the roxygen tags |
||
export(setinfo) | ||
export(xgb.Callback) | ||
export(xgb.DMatrix) | ||
|
Large diffs are not rendered by default.
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.
Not sure what's the implication of changing
,
to.
. I know this is generated, but not familiar enough with R type system to understand the change.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.
This pops up on my side since quite some months. I can easily undo this change.
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.
I'm okay with the changes as they are generated. I am just curious about the implications. We run doc gen on the CI as well, not sure what's the cause of the change.