-
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] [ci] Cover 100% of R-to-C++ calls with unit tests #2944
Comments
Closing this issue in favor of being in #2302 . If you'd like to address this issue, leave a comment here and it will be re-opened. |
I'm going to open this since we have an open PR for it and I'm working on the other PRs in the next few days |
All functions have been covered now that we've merged #3020, so this can be closed |
This issue 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. |
LightGBM
's R package calls its C++ library using R's.Call()
interface. That C++ is one codebase that is reused as a standalone C++ library, with Java, with Spark (via the Java interface), with Python, and from a command-line interface.Because that library is shared, it's regularly updated by developers who have no knowledge of the
lightgbm
R package. The R package's unit tests are the best defense against such updates having unintended consequences and breaking the R package.The following calls to the C++ side from within the R package are not covered by this project's unit tests (this will be updated as tests are added):
LGBM_BoosterFree_R
([R-package] Added unit tests on creating Booster from file or string #2945)LGBM_BoosterCreateFromModelfile_R
([R-package] Added unit tests on creating Booster from file or string #2945)LGBM_BoosterGetCurrentIteration_R
([R-package] Added unit tests on creating Booster from file or string #2945)LGBM_BoosterMerge_R
([R-package] Added tests on creating a Booster from a Dataset #3007)LGBM_BoosterLoadModelFromString_R
([R-package] Added unit tests on creating Booster from file or string #2945)LGBM_BoosterResetTrainingData_R
([R-package] added tests on LGBM_BoosterResetTrainingData_R #3020)LGBM_BoosterRollbackOneIter_R
([R-package] added tests on LGBM_BoosterRollbackOneIter_R #3006)LGBM_BoosterSaveModelToString_R
([R-package] Added unit tests on creating Booster from file or string #2945)LGBM_GetLastError_R
([R-package] Added tests on LGBM_GetLastError_R #3005)I figured that out by following the coverage instructions in the R-package README.
Covering those calls is important because it would improve our confidence in releases and allow non-R developers to work on the C++ library without worrying that their changes are negatively impacting the R package.
This issue will likely not be closed by a single PR. Any PR that addresses some of those functions above would be a welcome addition.
The text was updated successfully, but these errors were encountered: