Skip to content
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] deprecate lgb.prepare() and lgb.prepare2() #3095

Merged
merged 16 commits into from
Aug 1, 2020

Conversation

jameslamb
Copy link
Collaborator

Part of #3075 . lgb.prepare() and lgb.prepare2() have a lot of code overlap and it could be confusing to know which one to use.

To reduce the public API of the R package and make it easier for uses to understand how too get from their raw data to a trained model, this PR deprecates lgb.prepare() and lgb.prepare_rules() in favor oflgb.prepare2() and lgb.prepare_rules2() . They'll be removed completely in LightGBM 3.0.0 (#3071 )

@jameslamb jameslamb removed the request for review from StrikerRUS May 16, 2020 20:52
@jameslamb jameslamb force-pushed the r/deprecate-prepare branch from a26f7b7 to 937edfa Compare May 16, 2020 21:46
@jameslamb
Copy link
Collaborator Author

gently ping @Laurae2 for a review

Copy link
Contributor

@Laurae2 Laurae2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy-paste typo to fix for lgb.prepare_rules.

R-package/R/lgb.prepare_rules.R Outdated Show resolved Hide resolved
@jameslamb jameslamb force-pushed the r/deprecate-prepare branch from 937edfa to 51c349e Compare June 14, 2020 02:41
@jameslamb jameslamb requested a review from guolinke June 22, 2020 16:37
@guolinke
Copy link
Collaborator

@jameslamb please check my comments inline.

@jameslamb
Copy link
Collaborator Author

I just merged master into this to get the latest changes

@jameslamb
Copy link
Collaborator Author

I've merged in the changes from #3188 . This is ready for review.

Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad to see API simplification!

R-package/R/removed.R Outdated Show resolved Hide resolved
R-package/demo/categorical_features_rules.R Outdated Show resolved Hide resolved
R-package/tests/testthat/test_lgb.convert_with_rules.R Outdated Show resolved Hide resolved
R-package/tests/testthat/test_lgb.convert_with_rules.R Outdated Show resolved Hide resolved
R-package/tests/testthat/test_lgb.convert_with_rules.R Outdated Show resolved Hide resolved
R-package/tests/testthat/test_lgb.convert_with_rules.R Outdated Show resolved Hide resolved
Copy link
Contributor

@Laurae2 Laurae2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix description / comment, then we can merge whenever ready

R-package/R/lgb.convert.R Outdated Show resolved Hide resolved
R-package/tests/testthat/test_lgb.convert.R Show resolved Hide resolved
@jameslamb
Copy link
Collaborator Author

@StrikerRUS have you also seen this recently on AppVeyor?

image

@StrikerRUS
Copy link
Collaborator

have you also seen this recently on AppVeyor?

Nope...

@jameslamb
Copy link
Collaborator Author

have you also seen this recently on AppVeyor?

Nope...

ok, hopefully it was just something transient. I don't see any incidents on AppVeyor: https://status.appveyor.com/

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants