[R-package] Allow for valid data to be a slice of the main lgb.Dataset by setting references if one does not exist. #6844
Conversation
Slices of datasets already contain a reference to the base data (e.g., R/lgb.Dataset.R:562). This change only sets a reference to the base dataset if one does not already exist.
walkerjameschris
requested review from
guolinke,
jameslamb,
shiyu1994,
jmoralez,
borchero and
StrikerRUS
as code owners
jameslamb
changed the title
There was a problem hiding this comment.
Thanks for taking the time to contribute!
I've added a link to #6008 to the description... please do that when you contribute in the future and where your contribution is related to previous discussions. It helps to answer the question "what is the benefit of this change?" for reviewers.
Before we review this closely (I haven't yet), please also add a unit test covering this behavior. That'll help build our confidence in this fix, and prevent the bug from being reintroduced in the future.
Add that somewhere in https://github.com/microsoft/LightGBM/blob/master/R-package/tests/testthat/test_basic.R. Try to put it near related tests, and follow all the patterns you see there (for code style, passing .LGB_VERBOSITY. and .LGB_MAX_THREADS, etc.).
|
@microsoft-github-policy-service agree |
fixes #6008
Slices of datasets already contain a reference to the base data. This change only sets a reference to the base dataset if one does not already exist.