-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix PDF contents #21659
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 PDF contents #21659
Conversation
doc/users/installing/install.rst
Outdated
@@ -0,0 +1,3 @@ | |||
.. redirect-from:: /users/installing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This leaves /users/installing/index.html a blank page except for the heading "Installation" Did we want to un-hide the toctree?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm yea, that's annoying. The problem is that including INSTALL.rst
directly causes duplicate labels and other weirdness. I guess showing the toc might be the easiest fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think a problem here is the include. Seems like it would be preferable for INSTALL.rst to simply refer to the correct place in the documentation, rather than have canonical information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I ended up moving things into doc/users/installing.rst
for multiple reasons. Having it separate caused weirdness in the toctree of the PDF, moving it to INSTALL.rst
breaks the redirects, and putting things in doc/users/installing/install.rst
like I did before makes empty pages without tocs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to not have it in doc/users/installing/index.rst
? Other than the fact that maybe installing doesn't have any subfiles now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also used to be there in <=3.4, so it seemed best to move it back with it being only one file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but moving it was deliberate. Currently from https://matplotlib.org/stable/users/index under "General" each of the sections (Getting Started, Installation, etc) goes to its own subdirectory in users/getting_started
etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I can revert that. I wonder if we could end up keeping the file split, if we only got rid of the include
stuff, but this is probably fine.
This file was removed in matplotlib#21251
This order seems to make more sense in the PDF, and it's probably okay in the webpage too. The end of the Quick start (which is the first page of General points to Installation, Plot types, and Tutorials, so you can navigate to those parts that used to be first quite easily.
This ensures that the ToC as generated in the PDF makes sense, and doesn't appear in the wrong order. Also fixes the redirect to come from the right place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM if the PDF is good...
...by which I meant feel free to self merge if the Pdf is good. 😉 |
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free to suggest an improvement. |
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free to suggest an improvement. |
Backport PR #21659 on branch v3.5.x (Fix PDF contents)
Backport PR #21659 on branch v3.5.0-doc (Fix PDF contents)
PR Summary
This fixes:
contents
was removed in DOC: more site re-org #21251.Fixes #21655
PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).