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 AttributeError("'NoneType' object has no attribute 'get'") when including TOC #347

Merged
merged 8 commits into from
Mar 25, 2021

Conversation

tomas-pajurek
Copy link
Contributor

@tomas-pajurek tomas-pajurek commented Mar 24, 2021

When a page includes other page (via .. include::) containing a TOC (.. toctree::), the sphinx-build command fails with following error:

Theme error:
An error happened in rendering the page toc-extension.include.
Reason: AttributeError("'NoneType' object has no attribute 'get'")

Minimal repro can be found here.

I have traced the issue to recursive function add_header_level_recursive which receives None as an argument in the situation described in the repro above.

This MR fixes the issue by not calling add_header_level_recursive function with None argument.

Copy link
Collaborator

@bollwyvl bollwyvl left a comment

Choose a reason for hiding this comment

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

LGTM!

@bollwyvl
Copy link
Collaborator

bollwyvl commented Mar 24, 2021 via email

@jorisvandenbossche
Copy link
Member

@tpajurek-dtml thanks a lot for the fix!

Since you already constructed a minimal repro sphinx site, would you like to add that to our tests? You can add it as a new directory in https://github.com/pydata/pydata-sphinx-theme/tree/master/tests/sites and then add a new test function in https://github.com/pydata/pydata-sphinx-theme/blob/master/tests/test_build.py (and already ensuring it doesn't error might be sufficient)

@tomas-pajurek
Copy link
Contributor Author

@bollwyvl @jorisvandenbossche Sure, I will include the refactoring as well as the test.

@tomas-pajurek
Copy link
Contributor Author

It is done.

Copy link
Collaborator

@bollwyvl bollwyvl left a comment

Choose a reason for hiding this comment

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

L(even better)TM!

Copy link
Member

@jorisvandenbossche jorisvandenbossche 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 adding the test!
I just pushed a small change before merging this: I simplified the conf.py a bit to the bare content needed for the test (and removed the .gitignore, since in our testing machinery the build output is put in a temporary directory anyway).

@jorisvandenbossche jorisvandenbossche merged commit c36390f into pydata:master Mar 25, 2021
@jorisvandenbossche
Copy link
Member

And released 0.5.2 with this fix!

@tomas-pajurek
Copy link
Contributor Author

Great, thanks !

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.

3 participants