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] Updated lgb.train.R with keyword arguments #3452

Merged
merged 3 commits into from
Oct 13, 2020

Conversation

iadi7ya
Copy link
Contributor

@iadi7ya iadi7ya commented Oct 11, 2020

Fixes lgb.train.R from a list of functions listed on #3390

@iadi7ya
Copy link
Contributor Author

iadi7ya commented Oct 11, 2020

Hi @jameslamb. Can you please regenerate the docs and guide me how can I run the checks again. Thanks:)

Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! I've requested a few changes to this PR. In addition, can you please also add the appropriate keyword arguments for the following calls?

  • (line 250) categorize.callbacks()
  • (line 255) booster$set_train_data_name()
  • (line 259) booster$add_valid()

R-package/R/lgb.train.R Outdated Show resolved Hide resolved
R-package/R/lgb.train.R Outdated Show resolved Hide resolved
R-package/R/lgb.train.R Outdated Show resolved Hide resolved
@jameslamb
Copy link
Collaborator

Hi @jameslamb. Can you please regenerate the docs and guide me how can I run the checks again. Thanks:)

thanks so much for the pull request! I've left some requested changes. Once those have been made, I can come update the documentation.

@iadi7ya
Copy link
Contributor Author

iadi7ya commented Oct 12, 2020

Hi @jameslamb I have fixed the changes which you have suggested. Most of the checks have passed but in 3 of the checks I am getting following error:

The downloaded binary packages are in C:\Users\runneradmin\AppData\Local\Temp\RtmpuoWnIc\downloaded_packages
Downloading https://github.com/microsoft/LightGBM/releases/download/v2.0.12/miktexsetup-4.0-x64.zip
Setting up MiKTeX
miktexsetup.exe: The remote package repository is outdated. You have to choose another repository.
miktexsetup.exe: Data: url="https://ctan.math.illinois.edu/systems/win32/miktex/tm/packages/"

Is that a problem or is it ready for merge?

@jameslamb
Copy link
Collaborator

this happens sometimes and is definitely unrelated to your changes. I just restarted the builds, hopefully they'll pass.

@iadi7ya
Copy link
Contributor Author

iadi7ya commented Oct 12, 2020

Thanks @jameslamb for running the checks again. I hope it will pass all the check this time.

@jameslamb
Copy link
Collaborator

It looks like we have an ongoing issue with the MikTeX ecosystem (#3454 ). I'll merge this PR once I'm able to fix that, apologies.

@iadi7ya
Copy link
Contributor Author

iadi7ya commented Oct 13, 2020

Thanks @jameslamb :)

Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Ok now the tests are passing!

Thanks very much for contributing, @iadi7ya . I hope you'll consider contributing more in the future 😀

@jameslamb jameslamb merged commit 0c1c36c into microsoft:master Oct 13, 2020
@iadi7ya
Copy link
Contributor Author

iadi7ya commented Oct 13, 2020

That's awesome @jameslamb ! I'd love to keep contributing to this wonderful project :)

@iadi7ya iadi7ya deleted the patch-1 branch October 17, 2020 06:03
@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants