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

New Sphinx tutorial, part III #9534

Merged
merged 27 commits into from Oct 9, 2021

Conversation

astrojuanlu
Copy link
Contributor

@astrojuanlu astrojuanlu commented Aug 9, 2021

Feature or Bugfix

  • Feature

Purpose

This is a continuation of the work we did in #9276 and #9355. It expands some sections already started in the first part and adds some new ones, as originally proposed in #9165:

  1. Describing code in Sphinx
  2. Autogenerating documentation from code in Sphinx

In #9424 I started writing a section on intersphinx, but after some discussion I decided to postpone it a bit.

Also, as suggested by @tk0miya in #9424, this splits the tutorial into several documents, to make it more amenable.

Rendered version: https://sphinx--9534.org.readthedocs.build/en/9534/tutorial/index.html

cc @ericholscher

Relates

@astrojuanlu astrojuanlu marked this pull request as draft August 10, 2021 09:04
@astrojuanlu
Copy link
Contributor Author

test_autodoc_typehints_signature is failing, I guess it's unrelated.

@astrojuanlu
Copy link
Contributor Author

@tk0miya tk0miya added type:docs type:proposal a feature suggestion labels Aug 10, 2021
@tk0miya
Copy link
Member

tk0miya commented Aug 10, 2021

Could you divide this PR into two PRs; 1) separating a document into multiple files and 2) adding a new document, please? It helps to read the diff. I need to read the whole of the text if they're mixed.

@tk0miya
Copy link
Member

tk0miya commented Aug 10, 2021

FYI: The CI error will be fixed if you merge the HEAD of 4.x branch into your branch.

@astrojuanlu

This comment has been minimized.

@tk0miya
Copy link
Member

tk0miya commented Aug 10, 2021

I know. But it's a bit annoying if reviewing continues long. Additionally, no advantage to keeping them coupled.

@astrojuanlu
Copy link
Contributor Author

Separated the split in #9540, this should be reviewed after that.

@astrojuanlu astrojuanlu marked this pull request as ready for review August 12, 2021 15:16
@astrojuanlu
Copy link
Contributor Author

This is now ready for review!

@astrojuanlu
Copy link
Contributor Author

This warning is on me to fix, but again the filename has a double extension:

Warning, treated as error:
/home/runner/work/sphinx/sphinx/doc/tutorial/describing-code.rst.rst:303:line number spec is out of range(1-2): '3'

@astrojuanlu
Copy link
Contributor Author

Since nobody has started reviewing this yet, and in the interest of cleaning up the git history now that #9540 has been reviewed, I rebased and force pushed this.

@astrojuanlu
Copy link
Contributor Author

astrojuanlu commented Aug 16, 2021

Comment for future reviewers: I wonder if I should mention sphinx-apidoc somewhere, perhaps to complement (or even replace) the autosummary material I included.

@ericholscher
Copy link
Contributor

Comment for future reviewers: I wonder if I should mention sphinx-autoapi somewhere, perhaps to complement (or even replace) the autosummary material I included.

I don't believe so. It's not well-tested, and should be a bit more robust before it's in the official tutorial.

@astrojuanlu
Copy link
Contributor Author

Sorry, I meant the sphinx-apidoc command. Amending my previous comment.

@astrojuanlu
Copy link
Contributor Author

Reminder that this is ready for review ✔️

Copy link
Contributor

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

I got one of the files reviewed, at least. I ran out of energy before I got to the second one.

doc/tutorial/getting-started.rst Outdated Show resolved Hide resolved
doc/tutorial/describing-code.rst Outdated Show resolved Hide resolved
doc/tutorial/describing-code.rst Outdated Show resolved Hide resolved
doc/tutorial/describing-code.rst Outdated Show resolved Hide resolved
doc/tutorial/describing-code.rst Outdated Show resolved Hide resolved
doc/tutorial/describing-code.rst Outdated Show resolved Hide resolved
doc/tutorial/describing-code.rst Show resolved Hide resolved
doc/tutorial/describing-code.rst Show resolved Hide resolved
doc/tutorial/describing-code.rst Show resolved Hide resolved
doc/tutorial/describing-code.rst Outdated Show resolved Hide resolved
@astrojuanlu
Copy link
Contributor Author

I addressed the comments, and also added an explanation at the top stating that several languages are supported. This is ready for the final review.

Copy link
Member

@tk0miya tk0miya left a comment

Choose a reason for hiding this comment

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

LGTM.

@jakobandersen
Copy link
Contributor

jakobandersen commented Sep 14, 2021

The new text about the languages is great, but I'm still concerned about the structure. The current structure is

  • Describing code in Sphinx
    • Documenting Python objects
    • Cross-referencing Python objects
    • Including doctests in your documentation

so let's say someone writes a C version tomorrow, how should the structure be changed? Having more subsections under "Describing code in Sphinx" seems confusing, even if the doctests section gets "Python" in its title.
I'm not suggesting that there needs be changes towards this in this PR, but there I think there must a plan to avoid having to drastically change the Python part at that time.

@astrojuanlu
Copy link
Contributor Author

One idea could be to add a "Documenting other languages" subsection, or appendix, to this chapter. That way it doesn't "break the flow" of the current tutorial, but I doubt that it will be sufficient for avid C and C++ users.

Another idea would be to turn this chapter into a sort of "Choose your own adventure" thing, in which Lumache is either written in Python, C, or C++. Some minor wording modifications would be needed in the introduction, but apart from that, the prose would be almost the same.

And another idea would be to write a "Sphinx for C and C++ users" tutorial (I don't think there has to be one tutorial, or the tutorial - a tutorial is just another category of technical documentation) that assumes that the user has read the first sections of the Beginners tutorial (the one we're writing in this series of pull requests) and picks from there. That would require a bit more work, but possibly provide a more pleasant reading experience for C and C++ users.

@jakobandersen
Copy link
Contributor

One idea could be to add a "Documenting other languages" subsection, or appendix, to this chapter.

I'm not too keen on this. As a non-Python programmer you are then directed towards Python stuff you may not be interested in or need at all. And while Sphinx has an origin story with CPython, it is used much more widely now.

Another idea would be to turn this chapter into a sort of "Choose your own adventure" thing, in which Lumache is either written in Python, C, or C++. Some minor wording modifications would be needed in the introduction, but apart from that, the prose would be almost the same.

(Emphasis mine) Exactly! This is what I would most like. You have nicely written the tutorial such that this is possible (except the present structuring discussion), so why not reuse the rest of the tutorial for non-Python programmers?
Maybe the change when another language is added is simply change

  • Describing code in Sphinx
    • Documenting Python objects
    • Cross-referencing Python objects
    • Including doctests in your documentation

to

  • Describing code in Sphinx
    • Python
      • Documenting objects
      • Cross-referencing objects
      • Including doctests in your documentation
    • C
      • C stuff
    • C++
      • C++ stuff

and add the choose-your-adventure sentence? But perhaps that section nesting is too deep? That's why I think it should be thought through now rather than later.

And another idea would be to write a "Sphinx for C and C++ users" tutorial (I don't think there has to be one tutorial, or the tutorial - a tutorial is just another category of technical documentation) that assumes that the user has read the first sections of the Beginners tutorial (the one we're writing in this series of pull requests) and picks from there. That would require a bit more work, but possibly provide a more pleasant reading experience for C and C++ users.

I'm not sure that work would be worth it. I don't think there is anything language specific in the previous sections (except for may be intro notes in the beginning about the overall structure), so essentially duplicating them doesn't seem right to me.

@astrojuanlu
Copy link
Contributor Author

For the "choose your own adventure" option, we could use tabs, but perhaps some minor edits in the prose would be needed. Not too keen on adding 2 extra subsections.

For the last option, just to clarify, the first chapters wouldn't be repeated. The tutorial would announce that "if this is your first time with Sphinx, read the first N chapters of the basic tutorial before this one".

In any case, I'm a fan of YAGNI, and I'd rather not modify the present PR for something that is not there yet. We don't know if the C & C++ versions will come in the next month, or we will have to wait for 3 years...

@tk0miya
Copy link
Member

tk0miya commented Sep 14, 2021

I don't think it's important to describe all programming languages Sphinx supports in this tutorial. The important thing I think is letting the readers know 1) what domains can do (describing object and referencing them), 2) where the directives and roles are described, and 3) the difference of the languages.

This chapter only explains about the usage of :py:function: and :py:exectpion. There are no mentions for :py:class, :py:method: and so on. But, IMO, it's okay because this describes 1) and 2). I wish the users who read this tutorial will read the reference to describe other kinds of Python objects. Similary, I suppose the users using other languages will be able to get the right place of references if they read it.

But, this tutorial does not mention about 3). Autodoc and doctest processes only Python codes. Other languages are not supported. So it would be much better to explain the difference.

So I feel this would be better:

  • Describing code in Sphinx
    • Python
      • Documenting objects
      • Cross-referencing objects
      • Including doctests in your documentation
    • Other languages (C/C++/etc.)
      • Documenting objects

What do you think?

Note: I think it's not too late to make ToC extendable after merging. I'd like to merge this if we'll get agreed on the structure of ToC. It's okay to modify in another PR :-)

@astrojuanlu
Copy link
Contributor Author

For me this proposal is good, it's more or less aligned with the first idea I wrote in #9534 (comment), but with a better structure so it's more clear what differences are between the languages.

@jakobandersen
Copy link
Contributor

I like it as well, it's extensible without requiring large changes up front.

@astrojuanlu
Copy link
Contributor Author

We have a plan then? Is this ready to merge? :)

@tk0miya
Copy link
Member

tk0miya commented Sep 20, 2021

But, this tutorial does not mention about 3). Autodoc and doctest processes only Python codes. Other languages are not supported. So it would be much better to explain the difference.

Do you have a plan to change text about this (in another PR)?

@tk0miya
Copy link
Member

tk0miya commented Sep 20, 2021

Except above comment, I'm okay to merge this. @jakobandersen Do you have any opinion? If not, I'll merge this soon.

@astrojuanlu
Copy link
Contributor Author

Sure, count on it 👍🏽

@astrojuanlu
Copy link
Contributor Author

Friendly ping :)

@tk0miya
Copy link
Member

tk0miya commented Oct 9, 2021

Okay, let's go forward!

@tk0miya tk0miya merged commit 481c5e9 into sphinx-doc:4.x Oct 9, 2021
@astrojuanlu astrojuanlu deleted the new-tutorial-describing-code branch October 9, 2021 11:21
@astrojuanlu
Copy link
Contributor Author

Thanks! Setting a reminder to myself to address the changes discussed here.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type:docs type:proposal a feature suggestion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants