-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Sphinx generated documentation #521
Conversation
@Ir1d do you have practical experience with customization of |
@Borda I didn't modify the theme.. perhaps we need to maintain a fork by modifying |
@williamFalcon @Ir1d @neggert @jeffling could you pls have look and new docs... |
also @williamFalcon I would like to know what paths shall we use for doc/tutorial/blog/etc. |
Got everything checked out and building. Lots of warnings, but everything seems to have worked okay. First off, thanks for tackling this. This is a huge undertaking and it's going to be a great improvement to the package. A couple notes:
I don't want the perfect to be the enemy of the good, though. A lot of work has gone into getting this all set up, so maybe we just merge this with the understanding that it needs a bit more tweaking before we set RTD to public. |
I saw them too, but I wanted to get a feedback if this is the right way to go before I will invest more time to it...
Sure, depending on how large changes will we need we may need to fork the theme, but first I would need to know what links to keep and what to drop... @williamFalcon
yes, there is but it is the sam eas icon of an epic task in JIRA :)
I will have a look
I agree that we (me) shall get to stage that it is publishable, but then we shall merge it or move it to created |
the lightning logo is on the readme :) |
@Borda I see no problem. Lightning IS epic. ;) |
@neggert I have forked the PyTorch theme and did some adjustment but somehow the logo is not visible, it is probably something in CSS which I do not understand... Could you have look? |
Have you tried replacing this file directly? https://github.com/Borda/lightning_sphinx_theme/blob/master/images/logo.svg |
these images are irrelevant, you need to look at these inside the package... |
@williamFalcon @Ir1d @neggert @jeffling I would go for a merge, so pls could you do a review... |
Todo: ensure back compatibility... |
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.
Good stuff, only a few nitpicks. I did not read every single word on the actual docs, but we should do a quick review once this is deployed just for readability
@Borda rebase? |
yes, working on it right now, there are couple collisions... 😺 |
I'm following your instructions @Borda:
I'm getting these errors:
|
are you in the folder of correct Makefile (this PR)? seems you are in master branch |
CI builds without any problem https://circleci.com/gh/Borda/pytorch-lightning/298 |
formatting move docs from MD to docstring in particular package/modules formatting add Sphinx ext. rename root_module to core drop implicit name "_logger" drop duplicate name "overwrite" fix imports use pytorch theme add sample link mapping try fix RTD build use forked template fix some docs warnings fix paths add deprecation warnings fix flake8 fix paths revert refactor revert MLFlowLogger
Yup... still nothing.
|
hi @williamFalcon are you in master branch or this pr? |
this pr |
oh wait, i think it went back to master for some reason. hold on |
@Borda ok, got it to work. It looks like a good starting point but we need to work on the navigation since it is not intuitive at all right now nor obvious where to find things. Can we deploy this behind a dev endpoint to see what it looks like right now? Things that need to be fixed:
This is a good start, so I'm just going to highlight a few places we should focus on to get it to a good release point. I assume the people who look in places like this are people contributing to lightning, but we need to show it in a more user friendly way. This menu should have a Trainer and LightningModule root folder, then a kind of similar structure to the docs we have right now. Again, this is an amazing v1, so great job! Let's now move to make this more user friendly before we fully switch over to these docs. |
Talking about dev endpoint.. I'm suggesting netlify again.
I remember it involves some advanced usage of sphinx? Similar to what pytorch did..? Maintaining a root structure on our own instead of generating automatically? Not so familiar.. |
Not sure about his, it is a bit bond my experience so far...
What do you mean by logo, size?
for sure, I just wanted to keep the PR straight forward and draw the backbone so since this PR the work can be simpler divided (parallelize) into smaller PRs |
Oh I didn't notice this config before. @Borda Thx |
@Ir1d RTD allows you to have a separate version for each branch even for each tag/release |
@Borda for the record we use pl not pt import pytorch_lightning as pl |
typo 👼 and what about |
FYI, this is the fork we are right now using for docs - https://github.com/Borda/lightning_sphinx_theme |
Nice job bringing this in! |
Before submitting
What does this PR do?
Fixes #465.
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
The main changes:
root_module.root_module
->base_module.lightning
Watch
then open
pytorch-lightning/docs/build/html/index.html