-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
DOC Tutorial structure and layout #18261
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
DOC Tutorial structure and layout #18261
Conversation
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.
Thanks @cmarmo, the rendering does look better.
I'm not super experienced here, but I'm a little bit concerned about injecting raw html into the .rst
. I guess that's fine in the about
page because it won't change too much, but here we would have to handle the html code on top of the narrative docs which may become unfeasible.
Is there a way to handle this purely in sphinx? Maybe using the figclass
attribute of the figure? https://docutils.sourceforge.io/docs/ref/rst/directives.html#figure (again, I don't have a clear idea of what would work here)
If really that sounds too complex, then I'd suggest to drop the side-by-side layout and just have pictures and code aligned vertically.
Fine, I have removed all raw html calls. The layout is quite limited, but the the content will soon be modified so this is not an issue. |
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.
Thanks.
There seem to be some html left in supervised_learning.rst
I'm quite skeptical about the maintainability comment, because I don't find the website, as of today, easily maintainable in general.
I mean that mixing narrative docs + html is something that will hurt maintainability in the long run. As of today, we have zero html manually injected in our narrative docs, and I think that's for the better. I wasn't talking about the website. I definitely believe you if you say it's hard to maintain.
I would appreciate a quick merge or just a rejection. Thanks.
I'm doing my best by reviewing this @cmarmo. Thanks for your work so far.
EDIT sorry pressed enter too soon
Right, sorry. Fixed.
Understood, thanks for clarifying. |
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.
made a few comments, otherwise LGTM
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.
thanks @cmarmo !
Looks much better already
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Reference Issues/PRs
Towards #18257, uses #17865
What does this implement/fix? Explain your changes.
Fix unresponsive layout and general rendering of the tutorial.
Remove the 'Finding help' section.
Additional comments
I haven't addressed the TOC issue, I think this should be solved in a consistent way over the entire website (see also #16951)