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

Update documentation ahead of build-docs deprecation #2304

Merged
merged 15 commits into from
Feb 16, 2023

Conversation

stichbury
Copy link
Contributor

@stichbury stichbury commented Feb 9, 2023

Signed-off-by: Jo Stichbury jo_stichbury@mckinsey.com

NOTE: Kedro datasets are moving from kedro.extras.datasets to a separate kedro-datasets package in
kedro-plugins repository. Any changes to the dataset implementations
should be done by opening a pull request in that repository.

Description

This PR resolves #2114

Development notes

I removed all mention of build-docs except to refer back to it as legacy in the section about packaging a project (in the tutorial) I explained how to build docs with Sphinx instead. This was more painful than I anticipated.

After a few exploratory attempts, I decided the best way to guide readers wasn't to reproduce what we have been doing with Sphinx in build-docs but to advise users on on how to use Sphinx as they would if they weren't provided with any template. So I described how to use sphinx-quickstart with the latest version of Sphinx (otherwise it gets really tough since we use an old old version, and that itself means we have to fiddle with the version of Jinja2 and all kinds of other sh1t).

So I explain how to make a basic Sphinx config, how to add markdown and how to configure autodoc to build docstrings. I don't go into details of using the RTD theme or further extensions.

You can see the built version of the docs here: https://stichbury.github.io/tutorial/package_a_project.html


This can be committed now, once reviewed, but there is follow up work for Kedro 0.19. We need to remove the docs dependencies in setup.py:

"docs": [
            "docutils<0.18.0",
            "sphinx~=3.4.3",
            "sphinx_rtd_theme==0.5.1",
            "nbsphinx==0.8.1",
            "nbstripout~=0.4",
            "sphinx-autodoc-typehints==1.11.1",
            "sphinx_copybutton==0.3.1",
            "ipykernel>=5.3, <7.0",
            "Jinja2<3.1.0",
            "myst-parser~=0.17.2",
        ]

also we need remove the code for build-docs, and we need to update the project template/starter to remove the docs/source subfolder so there's no existing conf.py and index.rst to confuse things (then we can update these docs again).

I also think we should add basic documentation into the project template as to how to create docs (put a markdown folder in the /docs folder maybe) that echoes this advice on using Sphinx.

Note to reviewers

You can see the built version of these changes on my repo. The main changes are on this page.

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
…r level H1->H3

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury stichbury added the Component: Documentation 📄 Issue/PR for markdown and API documentation label Feb 9, 2023
@stichbury stichbury requested a review from merelcht February 9, 2023 16:16
@stichbury stichbury self-assigned this Feb 9, 2023
@astrojuanlu astrojuanlu self-requested a review February 9, 2023 17:49
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Copy link
Contributor Author

This has been a more painful PR than I had hoped, but it's ready for review now, and I'd appreciate your thoughts @idanov , @merelcht and @astrojuanlu. I am happy with my approach and think it's better to teach Sphinx basics but i'd love your opinions.

@stichbury stichbury marked this pull request as ready for review February 13, 2023 17:30
@stichbury stichbury requested a review from yetudada as a code owner February 13, 2023 17:30
@stichbury stichbury requested a review from idanov February 13, 2023 17:30
@astrojuanlu
Copy link
Member

You can see the built version of the docs here:

Notice we can enable the Pull Request Builder on RTD. For that, someone with proper permissions on the kedro-org/kedro repository needs to add a GitHub connection from https://readthedocs.org/dashboard/kedro/integrations/create/

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

Thanks a lot @stichbury, made a first round of comments.

Overall, I feel we could try to

  1. separate more clearly the advice for new projects vs migrating old ones,
  2. make the migration less scary or disruptive, and
  3. have a more clear stance towards MyST, even if it requires installing additional extensions

docs/source/tutorial/package_a_project.md Outdated Show resolved Hide resolved
docs/source/tutorial/package_a_project.md Outdated Show resolved Hide resolved
docs/source/tutorial/package_a_project.md Outdated Show resolved Hide resolved
docs/source/tutorial/package_a_project.md Outdated Show resolved Hide resolved
docs/source/tutorial/package_a_project.md Outdated Show resolved Hide resolved
docs/source/tutorial/package_a_project.md Outdated Show resolved Hide resolved
docs/source/tutorial/package_a_project.md Outdated Show resolved Hide resolved
@astrojuanlu
Copy link
Member

I now know what kedro build-docs does, will improve the feedback and send a restructuring proposal.

astrojuanlu and others added 3 commits February 15, 2023 12:29
- Put deprecation of `kedro build-docs` inside a warning admonition
- Clarify role of existing files and avoid deletion of `docs/source` contents
- Clarify that other documentation systems exist
- Make Sphinx commands forward and backward compatible
- Clarify steps for Markdown documentation
- Replace autosummary by regular autodoc

Co-authored-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Copy link
Contributor Author

stichbury commented Feb 15, 2023

@astrojuanlu Please could you take a final look through? I made a commit on top of yours just to revise the wording a little. I also changed the layout of the warning boxes to match those we use throughout our docs. Thanks again for your help on this, it was definitely 🤯

PS: HTML version is here https://stichbury.github.io/tutorial/package_a_project.html

Co-authored-by: Juan Luis Cano Rodríguez <hello@juanlu.space>
Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

This looks perfect to me now 💯 Thanks @stichbury!

Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

These docs are very clear and concise! I appreciate it must have been hard to document the setup this concisely, so fantastic work ⭐ Hopefully our users agree that this is easy to follow 🙂

docs/source/tutorial/package_a_project.md Outdated Show resolved Hide resolved
@stichbury stichbury merged commit ef081f5 into main Feb 16, 2023
@stichbury stichbury deleted the 2114-documentation-about-build-docs-update branch February 16, 2023 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Documentation 📄 Issue/PR for markdown and API documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update docs to remove build-docs and explain how to build project docs with Sphinx
3 participants