iiasa / ixmp

The ix modeling platform for integrated and cross-cutting scenario analysis
https://docs.messageix.org/ixmp
Apache License 2.0
36 stars 110 forks source link

Adapt to new RTD configuration style #491

Closed glatterf42 closed 11 months ago

glatterf42 commented 11 months ago

This PR aims to get our current RTD builds in line with the upcoming changes to the required configuration file and prevent future build issues due to dependency changes at the same time.

Looking into our docs settings in more detail, I found that the upcoming changes do not just mean that a RTD config file v2 is required, but some options that are now specified at -> Advanced Settings -> Default Settings will be removed in favor of options in the config file, which already overwrite them if such a file is present, even if they are not specified there. That should explain why no PDF builds are produced at the moment. So step 1 of this PR is to append the config file such that these options are active again.

Recently, our builds of the message_data docs have started to fail. The reason for that, as far as I can tell, is that the default pip installation process on RTD used to include upgrade-strategy eager, but recently changed to upgrade-strategy only-if-needed. This seems to be in line with popular demand. Since I couldn't find any mention in the docs on how to enable the upgrade strategy we are used to again (apart from taking complete control and specifying every step of the workflow ourselves), I decided it might be best to follow RTD's recommendation for reproducible builds by pinning the package versions we require. For internal consistency, I want to do so on ixmp, message_ix, message-ix-models, and message_data. Following the recommendation, I performed the following steps to reach the version of doc/requirements.txt present here:

  1. Activate venv, python -m pip install pip-tools
  2. pip-compile --allow-unsafe --extra docs -o doc/requirements.txt pyproject.toml
  3. Delete the lines following lines from the file manually:
    ixmp @ file:///home/fridolin/ixmp
    # via ixmp (pyproject.toml)

    Some notes on this process:

Finally, I still have to check and figure out how/why the bulleted lists are not rendering correctly. If this issue exists here as well, as I expect, I might use this PR to add commits with related fixes.

How to review

PR checklist

codecov[bot] commented 11 months ago

Codecov Report

Merging #491 (982cd52) into main (5c55eec) will not change coverage. The diff coverage is n/a.

@@          Coverage Diff          @@
##            main    #491   +/-   ##
=====================================
  Coverage   98.5%   98.5%           
=====================================
  Files         42      42           
  Lines       4506    4506           
=====================================
  Hits        4442    4442           
  Misses        64      64           
glatterf42 commented 11 months ago

I made the same mistake here as I did for #483: since this PR originates from my fork, I can't easily enable RTD builds for it. The same holds for iiasa/message_ix#733. Luckily, though, I don't (yet) use forks for message-ix-models and message_data, so I was able to build the docs for their PRs:

Both of them are looking good, in particular the bullet lists. Thus, I would trust the changes proposed here and for message_ix to have the same effect as they are essentially the same.

Note, though, that I wasn't able to render bullet lists correctly locally, despite updating all packages that I understood to be a dependency of sphinx to their versions suggested in doc/requirements.txt (this is for the message-ix-model build; the message_data one failed but see its PR for further details). I can try updating all packages or this issue might be related to my local css stylesheets being out-of-date, but that is an issue for another day.

khaeru commented 11 months ago

Thanks for all the details here. Without addressing all of them yet, one operational question comes up based on the tip in the RTD page on "Reproducible Builds" page:

Remember to update your docs’ dependencies from time to time to get new improvements and fixes. It also makes it easy to manage in case a version reaches its end of support date.

Namely: how will we know when we need to do this, and how can we minimize the friction involved? In the current process, we don't have to think about it at all or do anything (of course that leads to things like bullet lists mysteriously breaking without our notice, which it will be good to prevent).

I'm thinking that if we can automate the steps including "Delete the lines following lines from the file manually" (perhaps with a simple script that lives in doc/), we could automatically generate an updated version (for instance here) and then see when we need/want to commit it.

glatterf42 commented 11 months ago

We might not have to find a way to exclude these lines automatically. If we add a workflow step to create doc/requirements.txt on each run, the runner will have access to the local copy of the package that pip-compile will write into the file. However, it took a few minutes locally (<5) to create the files, so it might be good to avoid this; I will therefore check again to see if the lines for the package itself can be excluded easily.

glatterf42 commented 11 months ago

I've gathered several strings of information:

Why does doc/requirements.txt include the package itself?

According to their docs, pip-compile judges based on project.dependencies and project.optional-dependencies when called on a pyproject.toml file. E.g. for message-ix-models, these sections only contain a single reference to message-ix-model itself, namely line 58: "message_ix_models[tests]",. These nested dependencies cause the project to consider itself a dependency of itself. There doesn't seem to be any elegant way to mitigate this; maybe poetry could, but I'd have to check; simply stating all dependencies explicitly will cause noise. So there might not be an easy way to exclude the target lines from a pip-compile-created doc/requirements.txt automatically.

What about Poetry?

Tools like poetry might be able to control the dependencies in an elegant and modern way, but I'd have to look into it more. This might be worthwhile, though, as our stack will at least partly employ poetry with the new ixmp4. Is this something we might be interested in for the rest of the stack as well?

How can we truly enable PR builds for RTD?

What I activated for iiasa/message-ix-models#114 and iiasa/message_data#466 should probably not be called a 'PR build'. The RTD docs specify how to enable builds for PRs and this is not what I have done. I have activated builds for the branch fix/docs, but the project settings are still such that we don't build the html docs for all PRs. I'm also not sure we would want something like this, but I wanted to leave a note here. It should theoretically be possible to activate this option for a project and push a commit to a branch to trigger a docs-build for this PR.

How can we keep pinned dependencies up-to-date?

Last but not least, how do we want to keep the pinned packages up to date? RTD suggests

You do still need to upgrade your dependencies from time to time, but you should do that on your own schedule.

There are services to get notifications on new releases, but generally speaking, that does not seem to be considered best practice. It creates a lot of manual work and can be automated instead. Some people suggest using renovate, which is available on the GitHub marketplace and should be free, but it belongs to mend, which is not free in general, so I'm not entirely convinced. I could only install Renovate for myself or the whole IIASA organization, which don't seem like the best scopes here. One alternative that is integrated with GitHub is Dependabot. Dependabot is available for all repositories, but not necessarily all package management systems. All pip, pip-compile, and poetry are not supported by Dependabot on private repositories. It might suffice, however, to update the dependencies of message_data in sync with e.g. message-ix-models, though this would still need to be done manually. The latest version of pip that is supported is 21.1, while the latest one is 23.2, which might be relevant since pip only supports recursive (or nested, see above) dependencies since version 21.2. However, since we already have a requirements-file, we might not need to resolve the recursive dependencies. Apart from that, Dependabot allows to specify a dependency file that is regularly checked for possible upgrades. If some viable new combination is found, the bot creates a new PR, but a frequency can be specified to do so only daily/weekly/monthly. This might keep the extra workload manageable.

glatterf42 commented 11 months ago

As discussed earlier with @khaeru, I have added doc/requirements.in files to some of these PRs (all but this one, as it happens), in which I specified the package versions of the direct dependencies in the [docs] groups that are necessary to build the documentation. I also activated PR builds for message_ix temporarily and the (private) build can be found here: the bullet lists are looking good again :)

One other thing I noted: comparing the latest version of the message_data docs to the stable one, the bullet lists look fine on both (to my surprise) and a note on the top has gone missing. This is fixed by the fix/docs build again, but made me wonder: should we set the default version of our docs from 'stable' to 'latest'? The latest versions seem to be more reliable and accurate (cf the installation instructions for message_ix including the step to setup the mamba solver for anaconda). If I understand this correctly, the 'stable' version is built whenever something is pushed to a tag with a version number bigger than the one currently pointed to by 'stable'. For some reason, this includes PR builds (maybe it identifies the message_ix PR's 'version' as 733), so PR builds would be a way to update it. Other than that, only new tags will update 'stable', whereas each push to 'main' updates 'latest'. So if we publish tags/versions twice a year and introduce some changes to the docs soon after the last release (as was the case with the mamba solver), using 'stable' as our default version would mean manually telling people to use the updated 'latest' for roughly six months.

glatterf42 commented 11 months ago

Due to the success on the other PRs, this PR receives the same changes now. The steps for arriving at the current doc/requirements.in and doc/requirements.txt are as follows: Based on the existing doc/requirements.txt and pyproject.toml, I picked the versions of the dependencies needed for buildings the docs and wrote them to doc/requirements.in. The command to update doc/requirements.txt is then pip-compile --allow-unsafe -o doc/requirements.txt doc/requirements.in.

glatterf42 commented 11 months ago

The pinned dependencies need to be updated regularly, though not necessarily frequently. Since this topic is relevant across all four repositories, I decided to add information on why and how to the ECE wiki rather than any individual repo. In essence, we might start with manual updates in about four months, and if that seems to cumbersome in the long run, we can look into dependabot or some custom action instead.