[ci] [docs] use miniforge for readthedocs builds (fixes #4954) #4957

jameslamb · 2022-01-18T03:40:02Z

Fixes #4954.
Contributes to #4948.
Contributes to #4859.

This PR proposes updating LightGBM's readthedocs configuration to tell RTD to use mamba to manage dependencies. See the description of #4954 for relevant links to RTD documentation describing these new config options.

Notes for Reviewers

This PR updates the "check-docs" CI job to use conda. I'm hoping this can be merged before #4953, and then #4953 could switch that CI check to use mamba so LightGBM's CI will more closely match what happens on master RTD builds.

@StrikerRUS could you please temporarily enable builds for this branch on RTD so we can test the new configuration?

StrikerRUS · 2022-01-18T22:08:42Z

could you please temporarily enable builds for this branch on RTD so we can test the new configuration?

Sure, done!
https://readthedocs.org/projects/lightgbm/builds/15811372/

docs/README.rst

jameslamb · 2022-01-19T00:50:40Z

Sure, done!
https://readthedocs.org/projects/lightgbm/builds/15811372/

Thanks for that! I'm really happy to see that the build succeeded, docs look correct (I checked Python, C API, and the R docs), and that the build didn't take longer than usual.

StrikerRUS · 2022-01-20T00:54:26Z

Hmm... check-docs job failed with

Running Sphinx v4.4.0
making output directory... done
                                                                                             
Exception occurred:
  File "/home/runner/miniconda/envs/docs-env/lib/python3.9/site-packages/sphinx/theming.py", line 207, in load_external_theme
    theme_entry_points = entry_points(group='sphinx.html_themes')
TypeError: entry_points() got an unexpected keyword argument 'group'
The full traceback has been saved in /tmp/sphinx-err-bcgclosn.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
make: *** [Makefile:20: html] Error 2

https://github.com/microsoft/LightGBM/runs/4863199597?check_suite_focus=true

docs/build-docs.sh

docs/README.rst

Co-authored-by: Nikita Titov <[email protected]>

jameslamb · 2022-01-20T05:07:03Z

TypeError: entry_points() got an unexpected keyword argument 'group'

I think I understand what's happening.

I've found reports claiming that this error is about a difference in importlib-metadata versions.

TypeError: entry_points() got an unexpected keyword argument 'group' apache/airflow#19804 (comment)
In Twine, TypeError: entry_points() got an unexpected keyword argument 'group' python/importlib_metadata#331 (comment)

importlib.metadata (note the .) has been a part of the Python standard library since Python 3.8 (https://docs.python.org/3/library/importlib.metadata.html#entry-points).

importlib-metadata (note the -) is an external library providing importlib.metadata's functionality. From PyPI (link):

This package supplies third-party access to the functionality of importlib.metadata including improvements added to subsequent Python versions.

New features are introduced in this third-party library and later merged into CPython.

The conda-forge feedstock for importlib-metadata was archived on conda-forge in 2019 (https://github.com/conda-forge/importlib-metadata-feedstock).

It looks like sphinx will first try to use external package importlib_metadata if available, and otherwise will use the standardlib package.

https://github.com/sphinx-doc/sphinx/blob/9a42e42a401444dc833261409fb55a5067409b8e/sphinx/theming.py#L19-L22

try:  # Python < 3.10 (backport)
    from importlib_metadata import entry_points
except ImportError:
    from importlib.metadata import entry_points

I suspect that the issue here is related to mixing conda channels. Notice in https://github.com/microsoft/LightGBM/runs/4863199597?check_suite_focus=true that conda create is getting packages from the anaconda defaults channels, and then conda install is getting conda-forge channels.

python             pkgs/main/linux-64::python-3.9.5-h12debd9_4
readline           pkgs/main/linux-64::readline-8.1-h27cfd23_0
requests           pkgs/main/noarch::requests-2.25.1-pyhd3eb1b0_0
...
...
...
doxygen            conda-forge/linux-64::doxygen-1.9.2-hb166930_0
rstcheck           conda-forge/noarch::rstcheck-3.3.1-pyh9f0ad1d_0

I think that specifying nodefaults in env.yml is either not working as the documentation says it should or I've misunderstood that documentation (docs link).

You can exclude the default channels by adding nodefaults to the channels list.

I tried that approach because conda env create --file does not accept flags like --channel. I'm going to try pushing a commit with a different approach. Note that this issue should go away, and this code should be simplified, once we're using mamba[forge].

jameslamb · 2022-01-20T05:48:46Z

I haven't been able to figure out the right combination of conda commands to fix this issue.

Since the builds are working correctly on RTD (build link) and in Docker locally (both using mamba[forge]), I'm confident that just using mamba[forge] will resolve these issues.

So I'd like to request changing the order of merging PRs I'd originally suggested in this PR's description. @StrikerRUS could we merge #4953 before this one? Then I'll update this one to use mamba[forge].

StrikerRUS · 2022-01-22T20:27:20Z

So I'd like to request changing the order of merging PRs I'd originally suggested in this PR's description. @StrikerRUS could we merge #4953 before this one?

Sure!

jameslamb · 2022-02-11T01:08:27Z

Now that #4953 has been merged, I'll work on updating this.

https://docs.readthedocs.io/en/stable/config-file/v2.html#build-tools-python says it's possible to set python.version: miniconda3-4.7. I'll switch from mamba to that, based on #4953 (comment) and #4953 (comment)

jameslamb · 2022-02-16T04:26:12Z

Ok I think this is ready for review!

checks-docs build succeeded (build link)
- logs show all packages are being installed only from conda-forge
RTD build succeeded (build link)
- logs show all packages are being installed only from conda-forge
- build took 564s (most recent builds on master took 700+ seconds)
- Python package docs look correct: https://lightgbm.readthedocs.io/en/fix-rtd-configuration/Python-API.html
- long-form docs like Distributed Learning Guide look correct: https://lightgbm.readthedocs.io/en/fix-rtd-configuration/Parallel-Learning-Guide.html
- R docs were look correct: https://lightgbm.readthedocs.io/en/fix-rtd-configuration/R/index.html
I tested the docker run command introduced in docs/README.rst in this PR on my Mac tonight. It ran successfully and the docs rendered locally look correct.

StrikerRUS

Thank you so much for working on this! Please consider checking my fresh comments below.

.ci/test.sh

docs/README.rst

docs/build-docs.sh

StrikerRUS · 2022-02-16T22:14:40Z

docs/env.yml

+  - r-base=4.1.2
+  - r-data.table=1.14.2
+  - r-jsonlite=1.7.2
+  - r-knitr=1.37
+  - r-matrix=1.4_0
+  - r-pkgdown=1.6.1
+  - r-rmarkdown=2.11
+  - r-roxygen2=7.1.2


We don't want to pin R package versions anymore?.. #2176 (comment)

The comment you've linked refers to a commit, 014bee1, that changed the list of package from "not pinned at all" to "pin to aa specific BUILD of a specific VERSION".

This PR's changes are still pinning them to specific package VERSIONS, just not to individual BUILDS. Sometimes new builds of the same package version are pushed to conda channels, to fix issues like, for example, mistakes in metadata about declared dependencies. We should want to get such fixes automatically, as they make it more likely that the environment will solve correctly.

The comment you've linked refers to a commit, 014bee1, that changed the list of package from "not pinned at all" to "pin to aa specific BUILD of a specific VERSION".

More relevant past discussion: #2725 (comment).

OK, let's not pin build numbers for now. But if RTD starts to fail, we know what to try first.

Co-authored-by: Nikita Titov <[email protected]>

… into fix/rtd-configuration

docs/README.rst

StrikerRUS · 2022-02-18T01:54:33Z

docs/env.yml

+  - r-base=4.1.2
+  - r-data.table=1.14.2
+  - r-jsonlite=1.7.2
+  - r-knitr=1.37
+  - r-matrix=1.4_0
+  - r-pkgdown=1.6.1
+  - r-rmarkdown=2.11
+  - r-roxygen2=7.1.2


The comment you've linked refers to a commit, 014bee1, that changed the list of package from "not pinned at all" to "pin to aa specific BUILD of a specific VERSION".

More relevant past discussion: #2725 (comment).

OK, let's not pin build numbers for now. But if RTD starts to fail, we know what to try first.

StrikerRUS

Thanks a lot! Just two minor suggestions below and I think this PR can be merged.

Co-authored-by: Nikita Titov <[email protected]>

github-actions · 2023-08-23T14:10:53Z

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.

jameslamb added 6 commits January 16, 2022 22:57

[ci] [docs] use mamba for readthedocs builds (fixes #4954)

bbfa3e5

update docs

4a451bb

Merge branch 'master' into fix/rtd-configuration

945a1ba

simplify build script and add docs flag to gitignore

54b608d

exit with non-0 if build fails

5dd9c8e

update CI job

8df9337

jameslamb added the maintenance label Jan 18, 2022

jameslamb requested review from guolinke, shiyu1994 and StrikerRUS as code owners January 18, 2022 03:40

add doxygen

57a0537

StrikerRUS reviewed Jan 18, 2022

View reviewed changes

docs/README.rst Outdated Show resolved Hide resolved

remove outdated requirement_base.txt reference

d53e14e

jameslamb requested a review from StrikerRUS January 19, 2022 00:50

jameslamb added 3 commits January 18, 2022 22:13

use conda create instead of conda env create

e9aa5a1

fix conda create flags

343932f

add nodefaults to env.yml

8b90bbf

StrikerRUS reviewed Jan 20, 2022

View reviewed changes

docs/build-docs.sh Outdated Show resolved Hide resolved

docs/build-docs.sh Outdated Show resolved Hide resolved

docs/README.rst Outdated Show resolved Hide resolved

docs/README.rst Outdated Show resolved Hide resolved

Update docs/README.rst

fef18e6

Co-authored-by: Nikita Titov <[email protected]>

jameslamb added 2 commits January 19, 2022 23:32

try to fix check-docs CI job

0c312ee

additional changes

3efc619

jameslamb requested a review from StrikerRUS January 20, 2022 05:49

jtilly mentioned this pull request Jan 20, 2022

changed exact pins for minimum versions Quantco/glum#513

Merged

1 task

jameslamb mentioned this pull request Feb 4, 2022

[ci] [docs] readthedocs configuration options are deprecated #4954

Closed

StrikerRUS added the in progress label Feb 12, 2022

jameslamb added 4 commits February 12, 2022 20:55

Merge branch 'master' into fix/rtd-configuration

c923dfb

switch from mamba to miniforge

e88ba38

Merge branch 'master' into fix/rtd-configuration

dfda92f

simplify docker command and fix issues in local build script

ac037b5

jameslamb removed the in progress label Feb 16, 2022

StrikerRUS changed the title ~~[ci] [docs] use mamba for readthedocs builds (fixes #4954)~~ [ci] [docs] use miniforge for readthedocs builds (fixes #4954) Feb 16, 2022

StrikerRUS requested changes Feb 16, 2022

View reviewed changes

jameslamb and others added 4 commits February 16, 2022 19:30

Merge branch 'master' into fix/rtd-configuration

164581a

Apply suggestions from code review

8010f99

Co-authored-by: Nikita Titov <[email protected]>

Merge branch 'fix/rtd-configuration' of github.com:microsoft/LightGBM…

00b8e73

… into fix/rtd-configuration

update docs and conda

5faaa44

jameslamb requested a review from StrikerRUS February 17, 2022 02:40

StrikerRUS reviewed Feb 18, 2022

View reviewed changes

StrikerRUS approved these changes Feb 18, 2022

View reviewed changes

Apply suggestions from code review

3e66b3f

Co-authored-by: Nikita Titov <[email protected]>

StrikerRUS merged commit 2f27d4b into master Feb 19, 2022

StrikerRUS deleted the fix/rtd-configuration branch February 19, 2022 03:30

jameslamb mentioned this pull request Oct 7, 2022

[DO NOT MERGE] Release v3.3.3 #5525

Closed

40 tasks

github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ci] [docs] use miniforge for readthedocs builds (fixes #4954) #4957

[ci] [docs] use miniforge for readthedocs builds (fixes #4954) #4957

jameslamb commented Jan 18, 2022

StrikerRUS commented Jan 18, 2022

jameslamb commented Jan 19, 2022

StrikerRUS commented Jan 20, 2022

jameslamb commented Jan 20, 2022

jameslamb commented Jan 20, 2022

StrikerRUS commented Jan 22, 2022

jameslamb commented Feb 11, 2022

jameslamb commented Feb 16, 2022

StrikerRUS left a comment

StrikerRUS Feb 16, 2022

jameslamb Feb 17, 2022

StrikerRUS Feb 18, 2022

StrikerRUS Feb 18, 2022

StrikerRUS left a comment

github-actions bot commented Aug 23, 2023

[ci] [docs] use miniforge for readthedocs builds (fixes #4954) #4957

[ci] [docs] use miniforge for readthedocs builds (fixes #4954) #4957

Conversation

jameslamb commented Jan 18, 2022

Notes for Reviewers

StrikerRUS commented Jan 18, 2022

jameslamb commented Jan 19, 2022

StrikerRUS commented Jan 20, 2022

jameslamb commented Jan 20, 2022

jameslamb commented Jan 20, 2022

StrikerRUS commented Jan 22, 2022

jameslamb commented Feb 11, 2022

jameslamb commented Feb 16, 2022

StrikerRUS left a comment

Choose a reason for hiding this comment

StrikerRUS Feb 16, 2022

Choose a reason for hiding this comment

jameslamb Feb 17, 2022

Choose a reason for hiding this comment

StrikerRUS Feb 18, 2022

Choose a reason for hiding this comment

StrikerRUS Feb 18, 2022

Choose a reason for hiding this comment

StrikerRUS left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 23, 2023