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

Do we still need to remove conf.py and index.rst following build-docs removal #3376

Closed
Tracked by #3331
SajidAlamQB opened this issue Dec 1, 2023 · 9 comments
Closed
Tracked by #3331

Comments

@SajidAlamQB
Copy link
Contributor

SajidAlamQB commented Dec 1, 2023

Description

Following our work with the new docs tool it doesn't align with what #2304 has written out.

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).

Our templates still keep the conf.py and index.rst do we plan to remove these still?

As of now when we create a new project with the docs tool in order to get it working properly I have to pip install ".[docs]" which installs sphinx from pyproject.toml. Next we go into docs/source and move conf.py and index.rst to a backup location.
Then I can run sphinx-quickstart docs and replace the generated files with the backups and run make html to generate the docs in the project.

This was mostly following the instructions on our current stable docs: https://docs.kedro.org/en/stable/tutorial/package_a_project.html#build-html-documentation

  • conf.py and index.rst do we plan to remove these still?
  • If so what does the docs tool provide? just the requirements?
  • If not then documentation needs to be updated to reflect how users can get docs running properly via the tools selection. (We might need a better way then telling them to backup conf.py and index.rst and replacing, maybe some code that automates this?)

Cc: @stichbury @astrojuanlu @merelcht @AhdraMeraliQB

@stichbury
Copy link
Contributor

This is a tricky one because

Either:

  • We do provide the setup files for Sphinx, but it breaks sphinx quickstart for those that didn't use the docs flag at kedro new time. And not everyone will use it.

Or:

  • We don't provide those files, then anyone using kedro new --tools=docs will still have to do sphinx quickstart to get them.

@stichbury
Copy link
Contributor

I would suggest the less invasive option is to not provide the files. Can we update kedro new --tools=doc to itself make a call to sphinx quickstart for us, or cookiecutter those two files into the right place?

@SajidAlamQB
Copy link
Contributor Author

I would suggest the less invasive option is to not provide the files. Can we update kedro new --tools=doc to itself make a call to sphinx quickstart for us, or cookiecutter those two files into the right place?

We would need sphinx already installed beforehand unfortunately.

@stichbury
Copy link
Contributor

I think it's better to document that the user must call sphinx quickstart after they've done a kedro new --tools=docs since that's basically just following our existing docs so suggest we remove the conf.py and index.rst from the project template that we have added them to. However, I suggest you also as @astrojuanlu to consider this before you proceed.

@astrojuanlu
Copy link
Member

Two more ideas:

  • We do create those files themselves without Sphinx preinstalled. They can be really barebones, would be low maintenance. And we can use markdown instead ;)
  • We take a tool-agnostic approach and only populate docs/ with a README.md that says "your docs go here" with some prose recommending tools (Sphinx, MkDocs, nothing at all)

@stichbury
Copy link
Contributor

stichbury commented Dec 1, 2023

Do those ideas address the disconnect between readers that use the --tools=docs flag and those that don't?

If they use the flags and follow our documentation in the tutorial section, they'll get in a mess I think. Maybe we have to simply remove that section of our documentation just to avoid this .

@astrojuanlu
Copy link
Member

About the conf.py and index.rst, hence @SajidAlamQB's initial question, I think we have to follow the original plan:

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

And about the docs, I think telling users to do sphinx-quickstart docs would be okay for existing projects just because --tols=docs can only be used with kedro new for now. But then if we keep that, for new projects it's not at all clear what kedro new --tools=docs should do, hence the conflict you're describing if I understand correctly.

While we figure out a better solution, I'm supportive of removing that section from the docs. There's nothing Kedro-specific there, just a whirlwind tour of Sphinx that we're in charge of maintaining. In addition, it was always weird to have those instructions in the "Package an entire Kedro project" page - kedro package is about distributing the code (wheel + config), docs have nothing to do with it.

So, let's remove it and open a ticket to rebuild it in a "good SWE practices" kind of guide at some point, which can start as a blog post.

@astrojuanlu
Copy link
Member

astrojuanlu commented Dec 4, 2023

To summarise our offline discussion and amend my previous comment:

  • Even if we're not always happy with Sphinx, it's good that Kedro makes a specific recommendation (like we do with pytest for testing), so let's keep the Sphinx recommendation for now.
  • We were already providing a slightly different experience from sphinx-quickstart (no Makefile/make.bat, no empty dirs for templates and static, extra extensions) so we are keeping it for now.
    • We will open a separate issue to evaluate our setup at some point in the future.
    • There's a bigger question of "what are the documentation needs of data projects" that we could address.
  • We will clarify the docs so there's exactly two flows: either (a) the user has done kedro new --tools=docs, so they don't need to do sphinx-quickstart, or (b) they haven't, so they don't have a docs folder, and therefore they have to run sphinx-quickstart or create the files themselves.
    • We will then drop the warnings around backing up index.rst and so on.
    • We can frame this in a positive way, like "if you use kedro new --tools=docs you get some goodies, like predefined extensions etc".

@merelcht
Copy link
Member

merelcht commented Dec 11, 2023

Addressed in #3331

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

4 participants