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] fix warnings in unit tests #4225

Merged
merged 1 commit into from
Apr 26, 2021
Merged

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Apr 26, 2021

This PR addresses several (but not all) of the R unit test warnings documented in #4108.

Unknown parameter: valids / {memory_address}

[LightGBM] [Warning] Unknown parameter: 0x7fb4ee5c84d8>
[LightGBM] [Warning] Unknown parameter: valids

These warnings were caused by calls that were passing valids to lgb.cv().
lgb.cv() does not accept validation data...it creates holdout sets using k-fold cross-validation.

This is an example of something that is currently a warning which would be an error if ...
was not allowed (#4202 (comment), will write up a feature request soon #4226).

Unknown parameter: categorical_features

[LightGBM] [Warning] Unknown parameter: categorical_features

There are no meaningful features

[LightGBM] [Warning] There are no meaningful features, as all feature values are constant

This came from the test lgb.train() works with linear learners and data where a feature has only 1 non-NA value. That test should have added an additional feature with enough variability to be usable during training.

Notes for Reviewers

#4108 was originally opened as a good first issue about a month ago, with the hope that an outside contributor would pick it up. Normally I'd leave such an issue for non-maintainers. However, I want to focus on the R package over the next few weeks because of some issues that have been building up about it.

Given that I expect a lot of activity on the R package in the near future, I think it's very important that the R package's unit tests cover the package as reliably as possible.

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.

LGTM!

@StrikerRUS StrikerRUS merged commit 5014f19 into master Apr 26, 2021
@StrikerRUS StrikerRUS deleted the fix/r-test-warnings branch April 26, 2021 10:47
@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 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants