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

Solve issue with documentation not rendering properly #145

Merged
merged 1 commit into from
Jan 23, 2023

Conversation

dalonsoa
Copy link
Collaborator

Description

I think that Read The Docs install step is installing older version of some packages where the bug described this link - which is what we observe - is still present.

The solution should be as straight forward as to install the dependencies in the post_install step rather than when setting up the environment. This blog post describes de solution:

https://browniebroke.com/blog/specify-docs-dependency-groups-with-poetry-and-read-the-docs/

Fixes #143

Type of change

  • New feature (non-breaking change which adds functionality)
  • Optimization (back-end change that speeds up the code)
  • Bug fix (non-breaking change which fixes an issue)

Key checklist

  • Make sure you've run the pre-commit checks: $ pre-commit run -a
  • All tests pass: $ poetry run pytest

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

@dalonsoa dalonsoa changed the title Update .readthedocs.yaml Solve issue with documentation not rendering properly Jan 23, 2023
@dalonsoa dalonsoa requested a review from davidorme January 23, 2023 13:25
@codecov-commenter
Copy link

Codecov Report

Merging #145 (6fb4822) into develop (6f2da0d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #145   +/-   ##
========================================
  Coverage    94.06%   94.06%           
========================================
  Files           11       11           
  Lines          472      472           
========================================
  Hits           444      444           
  Misses          28       28           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@davidorme davidorme left a comment

Choose a reason for hiding this comment

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

Sounds good. Lets try it and see!

@davidorme davidorme merged commit 39aff5a into develop Jan 23, 2023
@davidorme davidorme deleted the dalonsoa-patch-1 branch January 23, 2023 13:55
@davidorme davidorme restored the dalonsoa-patch-1 branch January 23, 2023 14:02
@davidorme
Copy link
Collaborator

That does not solve the issue:
https://virtual-rainforest.readthedocs.io/en/latest/

Obviously that link will go out of date, but has no bullet points, but should. The RTD build page for that version shows a number of steps appearing between our post_create_environment and post_install steps:

https://readthedocs.org/projects/virtual-rainforest/builds/19247222/

One of those is a duplicate of the poetry install --with docs step. I wonder if that is fouling it up?

git clone --no-single-branch --depth 50 https://github.com/ImperialCollegeLondon/virtual_rainforest.git .
git checkout --force origin/develop
git clean -d -f -f
asdf global python 3.9.15
python -mvirtualenv 
pip install poetry
poetry config virtualenvs.create false
python -m pip install --upgrade --no-cache-dir pip setuptools<58.3.0
python -m pip install --upgrade --no-cache-dir pillow mock==1.0.1 alabaster>=0.7,<0.8,!=0.7.5 commonmark==0.9.1 recommonmark==0.5.0 sphinx sphinx-rtd-theme readthedocs-sphinx-ext<2.3
poetry install --with docs
poetry install --with docs
poetry run python -m ipykernel install --user --name=vr_python3
cat docs/source/conf.py
python -m sphinx -T -E -b html -d _build/doctrees -D language=en . _build/html

@davidorme
Copy link
Collaborator

I've seen a few comments about RTD pinning sphinx based on project age, but I think that is now outdated? For example, here: readthedocs/readthedocs.org#8679

@dalonsoa
Copy link
Collaborator Author

Is there any way of getting a more verbose log from RtD to find out exactly what is being installed?

@davidorme
Copy link
Collaborator

If you can access that build page, then you can click on the individual step lines to unfold them? It definitely looks like it is changing sphinx versions.

@dalonsoa
Copy link
Collaborator Author

I've resumed working on this and it looks to me that things now work as expected? The latest build of the develop branch (3 days ago) have all the bullet points and I see no errors in the build process.

I am not sure if it was my doing or something else, but it seems that the issue is solved.

For the data_system branch, you get as output of the last installation step:

(installing stuff...)

myst-parser 0.18.1 requires sphinx<6,>=4, but you have sphinx 6.1.3 which is incompatible.
myst-nb 0.16.0 requires sphinx<6,>=4, but you have sphinx 6.1.3 which is incompatible.
Successfully installed commonmark-0.9.1 docutils-0.19 mock-1.0.1 readthedocs-sphinx-ext-2.2.0 recommonmark-0.5.0 sphinx-6.1.3 sphinx-rtd-theme-0.5.1

While for the develop branch you don't get those errors, to start with:

(installing stuff...)

Successfully installed Jinja2-3.1.2 MarkupSafe-2.1.2 Pygments-2.14.0 alabaster-0.7.13 babel-2.11.0 commonmark-0.9.1 docutils-0.19 imagesize-1.4.1 mock-1.0.1 pillow-9.4.0 pytz-2022.7.1 readthedocs-sphinx-ext-2.2.0 recommonmark-0.5.0 snowballstemmer-2.2.0 sphinx-6.1.3 sphinx-rtd-theme-0.5.1 sphinxcontrib-applehelp-1.0.4 sphinxcontrib-devhelp-1.0.2 sphinxcontrib-htmlhelp-2.0.0 sphinxcontrib-jsmath-1.0.1 sphinxcontrib-qthelp-1.0.3 sphinxcontrib-serializinghtml-1.1.5

But, on top of that, this installation is followed by poetry properly installing the correct versions of the packages as in the lock file, so I think this post_install thing indeed solves the problem and ensures that the versions in RtD are the ones we want.

@davidorme
Copy link
Collaborator

Hmm - that does look resolved. feature/data-system doesn't have the patch applied, so looks like it is probably still using the RTD installed latest sphinx (6.1.3) and getting errors. On the develop branch, the post install is updating sphinx from 6.1.3 to 5.3.0 in that post-install step. So yup, all looks well now.

@davidorme davidorme deleted the dalonsoa-patch-1 branch January 27, 2023 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sphinx not rendering bullets from RST correctly
3 participants