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] [docs] fix warnings in pkgdown site building #3086

Merged
merged 6 commits into from
May 16, 2020

Conversation

jameslamb
Copy link
Collaborator

This PR fixes the warnings from the last outstanding item in this comment: #1143 (comment)

Warning message:
Topics missing from index:
* lgb_shared_params
* lightgbm

To confirm, I ran:

Rscript build_r.R
cd lightgbm_r
Rscript -e "pkgdown::build_site(install = FALSE)"

re-using this branch from #3072 since it's already enabled on readthedocs.

@jameslamb jameslamb added the doc label May 15, 2020
@jameslamb jameslamb requested a review from StrikerRUS May 15, 2020 03:40
@jameslamb
Copy link
Collaborator Author

oh @StrikerRUS was fix/remove-devtools already disabled at readthedocs? Do you mind re-enabling it?

@jameslamb jameslamb changed the title [R-package] [docs] fix warnings in pkgdown sitte building [R-package] [docs] fix warnings in pkgdown site building May 15, 2020
@StrikerRUS
Copy link
Collaborator

@jameslamb

oh @StrikerRUS was fix/remove-devtools already disabled at readthedocs? Do you mind re-enabling it?

Yeah, I removed the branch and disabled RTD builds right after merging previous PR. Sorry! Just enabled builds again.

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.

I can confirm that the warning has gone from build logs and lightgbm() - Train a LightGBM model item has appeared in the R API reference.

Cool, thanks!

@StrikerRUS
Copy link
Collaborator

All recent macOS R-package builds at Travis are stuck at

* checking whether package ‘lightgbm’ can be installed ...

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

Hope it is temporary problem...

@jameslamb
Copy link
Collaborator Author

@jameslamb

oh @StrikerRUS was fix/remove-devtools already disabled at readthedocs? Do you mind re-enabling it?

Yeah, I removed the branch and disabled RTD builds right after merging previous PR. Sorry! Just enabled builds again.

no problem! I should have asked you to keep them.

@jameslamb
Copy link
Collaborator Author

All recent macOS R-package builds at Travis are stuck at

* checking whether package ‘lightgbm’ can be installed ...

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

Hope it is temporary problem...

😬 I just tried rebuilding, let's see

@StrikerRUS
Copy link
Collaborator

I should have asked you to keep them.

OK!

😬 I just tried rebuilding, let's see

Already tried 3 times with master... 😢

@jameslamb
Copy link
Collaborator Author

😬 Travis status looks fine, no ongoing incidents... https://www.traviscistatus.com/

@jameslamb
Copy link
Collaborator Author

Just added 89c6b3c to see if we can get better logs and identify what is breaking in the Mac R environment on Travis. I'll revert that commit before we merge this, just using it for investigation.

@StrikerRUS
Copy link
Collaborator

Just added 89c6b3c to see if we can get better logs and identify what is breaking in the Mac R environment on Travis.

... and it just passed all checks! 😄

I guess building without logs is just near 10mins.
Probably we need some dummy output to prevent Travis from terminating our build:

https://docs.travis-ci.com/user/common-build-problems/#build-times-out-because-no-output-was-received

travis-ci/travis-ci#4190 (comment)

@jameslamb
Copy link
Collaborator Author

WOW! I wonder if this is somehow related to how we switched all print() to message() in install.libs.R?

@jameslamb
Copy link
Collaborator Author

I think it was the print() to message() change in #2965 . Looks like print() prints to stdout and message() to stderr:

image

@jameslamb
Copy link
Collaborator Author

I documented the issue with timeouts in #3091, and just added a PR for it in #3092

@StrikerRUS
Copy link
Collaborator

Great investigation!
Can we get back to print() for the sake of speed?

@jameslamb
Copy link
Collaborator Author

Great investigation!
Can we get back to print() for the sake of speed?

Sorry, I wasn't clear....I don't think our Mac builds are slower now because of message(). I was thinking that since message() prints to stderr, maybe changing print() to message() increased the likelihood that we'd go 10 minutes with no new stdout.

I think that we should:

  • keep message() in install.libs.R, agreeing with @Laurae2 that it's the more appropriate choices for package installation messages
  • separately investigate why the Mac builds for the r-package have gotten so much slower in the last few days

@jameslamb jameslamb requested a review from huanzhang12 as a code owner May 16, 2020 18:12
@jameslamb jameslamb force-pushed the fix/remove-devtools branch from ac9c4ab to 5304479 Compare May 16, 2020 18:15
@jameslamb
Copy link
Collaborator Author

I just rebased to master to get the fix from #3092 . Assuming this builds successfully, I'll merge it and then work on replacing build_r_site.R per #1143 (comment)

@jameslamb jameslamb merged commit 58e9b1d into master May 16, 2020
odimka pushed a commit to odimka/LightGBM that referenced this pull request May 17, 2020
)

* [R-package] [docs] fix warnings in pkgdown sitte  building

* trying to get better Travis logs

* trying to get better Travis logs

* undo testing fix

* undo testing fix

* empty commit
ChipKerchner pushed a commit to ChipKerchner/LightGBM that referenced this pull request Jun 10, 2020
)

* [R-package] [docs] fix warnings in pkgdown sitte  building

* trying to get better Travis logs

* trying to get better Travis logs

* undo testing fix

* undo testing fix

* empty commit
ChipKerchner pushed a commit to ChipKerchner/LightGBM that referenced this pull request Jun 11, 2020
)

* [R-package] [docs] fix warnings in pkgdown sitte  building

* trying to get better Travis logs

* trying to get better Travis logs

* undo testing fix

* undo testing fix

* empty commit
@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.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants