Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Fix PDF contents#21659
Uh oh!
There was an error while loading.Please reload this page.
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 includingINSTALL.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 intodoc/users/installing.rst for multiple reasons. Having it separate caused weirdness in the toctree of the PDF, moving it toINSTALL.rst breaks the redirects, and putting things indoc/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 indoc/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 fromhttps://matplotlib.org/stable/users/index under "General" each of the sections (Getting Started, Installation, etc) goes to its own subdirectory inusers/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 weonly got rid of theinclude stuff, but this is probably fine.
This file was removed inmatplotlib#21251
This order seems to make more sense in the PDF, and it's probably okayin the webpage too. The end of the Quick start (which is the first pageof General points to Installation, Plot types, and Tutorials, so you cannavigate to those parts that used to be first quite easily.
This ensures that the ToC as generated in the PDF makes sense, anddoesn'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 tosuggest 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 tosuggest 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)
Uh oh!
There was an error while loading.Please reload this page.
PR Summary
This fixes:
contentswas removed inDOC: more site re-org #21251.Fixes#21655
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand 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).