[R-package] only warn about early stopping and DART boosting being incompatible if early stopping was requested

[serkor1]

Fixes #6612

• Fix: The associated functions have been fixed by adding a nested if-statement that only triggers if the dart-booster is specified with early_stopping_rounds != NULL.

• Unittests: The unit-test in test_parameters.R have been rewritten in the following way

  • The lightgbm()-test have had a expect_no_warning()-clause added if the model is correctly specified.

  • A new unittest have been created that checks wether lgb.cv() throws a warning errorneously.

This commit closes #6612. 🔧

[serkor1]

@microsoft-github-policy-service agree

[jameslamb]

Thanks so much!

I've left some suggestions.

• I've changed the title of this.
  • PR titles become changelog notes here, see for example https://github.com/microsoft/LightGBM/releases/tag/v4.5.0
  • so the title should describe the change in a way that answers the question "what changed about LightGBM?", not "which files did this touch?"
• I've added "fixes [R-package] [BUG] Using lgb.train() with booster = "dart" gives warning regardless of early_stopping_rounds value #6612" to the description
  • see https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
• I've left some suggestions as comments directly on the diff. Please apply them directly in the browser, by clicking "Add to batch" and then commiting all of them.
  • see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/incorporating-feedback-in-your-pull-request#applying-suggested-changes

Other notes

If you push additional commits, please don't force-push. All commits here are merged into 1 when a pull request is merged, so force-pushing is unnecessary.

In the future when you contribute here (or in any open source project on GitHub), don't use the target branch (in this case master) on your fork. That makes it more difficult for maintainers to collaborate with you and makes it hard to keep your fork up to date with the upstream repo as it changes. Next time, create a separate branch on your fork, e.g. git checkout -b fix/r-dart-warning.

[jameslamb]

regarding this comment: #6612 (comment)

I am running into the following R CMD check-issue: Error: could not find function "expect_no_warning" on Windows R 3.6.X

I guess you're referring to https://github.com/microsoft/LightGBM/actions/runs/10455058311/job/28949296216?pr=6619? In the future, if you're referring to something we could observe by clicking links, please share those links in your comment.

There (r-package (windows-latest, MINGW, R 3.6, cmake) job), I see the following:

  -- 2. Error (test_parameters.R:168:7): lgb.cv() should only warn if you use 'dar

  Error: could not find function "expect_no_warning"

Maybe the version of {testthat} getting installed there is too old, I'm not sure. You could try looking into the version being installed and try to figure out when expect_no_warning() was added to {testthat}.

Alternatively, you could change from expect_no_warning() to expect_silent(), which I know has been in {testthat} for many years.

[serkor1]

Hi @jameslamb,

It took a while to finalize this PR - for some reason I couldn't get the testing environment up and running locally. In any case, I have used expect_silent() which works on Windows and other OS's too - so that was a good idea!

The main difference from your initial review is that I have shortened the description on the unittests as {lintr} gave me an error on the line length. This can be seen in 2a866e8.

I thought this would be an easy and fast fix, so I didn't consider creating a new branch. I am sorry for this - I will never do stuff on main anymore, I promise 🤞

And thank you so much for all your feedback on this PR.

Best,

[jameslamb]

My only other suggestions after reviewing again were minor style things that are all personal preference, so I just pushed a commit directly changing those. We'll merge this once CI passes.

Thanks so much for the help! We appreciate the report and you being willing to come contribute. We'd be happy to have you come back and contribute again any time 😊