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

Fix issue with building Sphinx documentation with recent Jinja version #2039

Merged
merged 1 commit into from
May 20, 2021

Conversation

hakonsbm
Copy link
Contributor

Fixes #2038 by adding a requirement to keep Jinja version below 3 when creating a Conda environment. Also adds the requirement for Read the Docs.

Current Sphinx versions do not support Jinja version 3.
@hakonsbm hakonsbm added T: Bug Wrong statements in the code or documentation S: Critical Needs to be addressed immediately I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels May 18, 2021
@hakonsbm hakonsbm added this to the NEST 3.0 milestone May 18, 2021
@hakonsbm hakonsbm added this to In progress in Documentation via automation May 18, 2021
@jessica-mitchell jessica-mitchell self-requested a review May 18, 2021 12:25
Copy link
Contributor

@terhorstd terhorstd left a comment

Choose a reason for hiding this comment

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

I am very unhappy with adding transitional dependencies to the nest-dev environment. Now it looks like NEST depends on an old version of Jinja2, where actually this is a bug in the Sphinx package not stating its dependencies correctly and NEST itself doesn't have any direct interest in Jinja2 at all.

For the time being I agree to add it, in order to prevent everyone's doc generation workflow to break. I could imagine this gets fixed upstream in sphinx/nbformat soon and we should remove the whole jinja2 dependency again. (please create an issue)

Documentation automation moved this from In progress to Review May 18, 2021
@hakonsbm
Copy link
Contributor Author

@terhorstd I agree that it's not optimal. Like I wrote in the issue, Jinja 3.0 support will be added with Sphinx 4.1.0, which, according to their milestones, should be finished June 12.

Copy link
Contributor

@jessica-mitchell jessica-mitchell left a comment

Choose a reason for hiding this comment

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

Thanks for catching this @hakonsbm. I just want to point out, at least up to now there have been no problems building on RtD, but since a fresh install of the conda environment includes jinja2 3.0 I agree with @terhorstd that this should be approved to not break anyone's workflow.

@jougs jougs merged commit 41ebec8 into nest:master May 20, 2021
Documentation automation moved this from Review to Done May 20, 2021
@hakonsbm hakonsbm deleted the fix_sphinx_jinja_issue branch May 21, 2021 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Critical Needs to be addressed immediately T: Bug Wrong statements in the code or documentation
Projects
Status: Done
Documentation
  
Done
Development

Successfully merging this pull request may close these issues.

Generating Sphinx documentation fails with an error after recent Jinja update
4 participants