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 broken plots in pkgdown site (fixes #3276) #3508

Merged
merged 3 commits into from
Nov 1, 2020

Conversation

jameslamb
Copy link
Collaborator

#3276 documents some issues rendering plots in examples for the R package's documentation site, at https://lightgbm.readthedocs.io/en/latest/R/reference/.

I think that this is caused by the use of an older R version. This PR proposes fixing that by upgrading to R 4.0.3. It seems that R 4.0.3 packages are now available from conda-forge (thanks to the hard work of @xhochy and other https://github.com/conda-forge/r-base-feedstock maintainers).

This PR also simplifies our setup on RTD in a few ways:

  • removes unnecessary dependency testthat
  • removes unnecessary setting of repos option
  • builds the entire env in one step (instead of installing anaconda and conda-forge packages separately)

Notes for Reviewers

I tested locally with the following code, on an Ubuntu 18.04 laptop using conda 4.8.4:

conda create \
    -q \
    -y \
    -c conda-forge \
    -n r_env \
        cmake=3.14.0=h52cb24c_0 \
        r-base=4.0.3=hc603457_2 \
        r-data.table=1.13.2=r40h0eb13af_0 \
        r-jsonlite=1.7.1=r40hcdcec82_0 \
        r-matrix=1.2_18=r40h7fa42b6_3 \
        r-pkgdown=1.6.1=r40h6115d3f_0 \
        r-roxygen2=7.1.1=r40h0357c0b_0

export R_LIBS="$CONDA_PREFIX/lib/R/library"
Rscript build_r.R
cd R-package
Rscript -e "roxygen2::roxygenize(load = 'installed')"
Rscript -e "pkgdown::build_site( \
        lazy = FALSE \
        , install = FALSE \
        , devel = FALSE \
        , examples = TRUE \
        , run_dont_run = TRUE \
        , seed = 42L \
        , preview = FALSE \
        , new_process = TRUE \
    ) \
    "

At least locally, the plots look ok now

image

image

@jameslamb
Copy link
Collaborator Author

@StrikerRUS I created this from a LightGBM branch so we can test. Can you please enable RTD builds for it?

Thank you!

@StrikerRUS
Copy link
Collaborator

@jameslamb

Can you please enable RTD builds for it?

Sure, done!

@StrikerRUS
Copy link
Collaborator

@jameslamb
Very nice indeed! Could you please bump CMake version in this PR as well as you are updating all installed packages?

@jameslamb
Copy link
Collaborator Author

@jameslamb
Very nice indeed! Could you please bump CMake version in this PR as well as you are updating all installed packages?

sure!

@jameslamb
Copy link
Collaborator Author

upgraded to 3.18.2 in 22f9dcb, let's see if it works

@jameslamb
Copy link
Collaborator Author

jameslamb commented Nov 1, 2020

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.

Great! Thanks for the fix!

@StrikerRUS StrikerRUS merged commit cf69591 into master Nov 1, 2020
@StrikerRUS StrikerRUS deleted the fix/r-plots branch November 1, 2020 20:50
@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