-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
WIP: [R-package] require lgb.Dataset, remove support for passing 'colnames' and 'categorical_feature' for lgb.train() and lgb.cv() #6714
base: master
Are you sure you want to change the base?
Conversation
…ing 'feature_name' and 'categorical_feature' to lgb.train() and lgb.cv()
Setting up a CI job to run reverse dependency checks is taking too long (#6734), let's not block this on that. Tomorrow, I will just test all the reverse dependencies manually. I won't merge this until I've done that and posted a comment here summarizing that. But opening this up now for review... @StrikerRUS or @jmoralez could you review? This + #5010 are the last 2 things I'd like to try to get into v4.6.0, and as a reminder we only have 5 more days to put up a v4.6.0 release on CRAN (#6791). Neither of those is critical, so if you do not have time to review please just let me know and we can skip them for this release. |
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, thank you for removing deprecated args!
Listing out the reverse dependencies (from https://cran.r-project.org/web/packages/lightgbm/index.html). I'll update this comment as I run the tests (probably tomorrow, my timezone, at this point, sorry). Reverse Imports
Reverse Suggests
Reverse Enhances
|
I like these changes,thanks for working on this. |
Fixes #6435
Enforces deprecations that have been in
{lightgbm}
sinvce v4.4.0:lgb.Dataset
todata
argument oflgb.cv()
colnames
andcategorical_feature
arguments fromlgb.train()
andlgb.cv()
Notes for Reviewers
Still need to do reverse dependency checks
Before this is merged, we should check that it won't break any projects on CRAN that depend on
{lightgbm}
. I might propose introducing a comment-triggered CI job we can use for this purpose, maybe with https://github.com/r-lib/revdepcheck.xgboost
did something similar recentlyAs @mayer79 mentioned on #6435. See dmlc/xgboost#10031