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

Beginning of new Sphinx tutorial #9276

Merged
merged 18 commits into from Jun 13, 2021
Merged

Conversation

astrojuanlu
Copy link
Contributor

@astrojuanlu astrojuanlu commented May 26, 2021

Feature or Bugfix

  • Feature

Purpose

This is the first pull request (hopefully out of many more to come) for the new Sphinx tutorial proposed in #9165. Paraphrasing @evildmp, it is by no means finished, but almost complete 🙂

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

Since this is my first substantial contribution to the Sphinx project, I wanted to open this small pull request early to address any potential disagreements the Sphinx maintainers might have about things like the tone, the structure, the link on the front page, or anything else.

This tutorial partially covers sections 2-4 of the table of contents originally proposed in #9165:

  1. About this tutorial
  2. Getting started
  3. First steps to document our project using Sphinx

There is room for those sections to grow in subsequent pull requests, they are not cast in stone. For now, I wanted to set the overall intent and structure of the whole tutorial. As a result, I might slightly change or reword these contents in the near future, to accommodate for its growth.

Some noteworthy decisions reflected implicitly or explicitly in the discussion in #9165:

  • Inclusion of the Tutorial in the front page
  • Tutorial "aimed towards Sphinx newcomers"
  • Assumes Linux-like command line. With some work, we could use tabs for different operative systems like the Django documentation does, but I didn't want to overcomplicate things just yet.
    • Still, we might want to offer a hint to macOS users (to reassure them that their terminal is fine) and to Windows users (to encourage them to use WSL2?)
  • Explicit usage of Python virtual environments, instead of other installation methods.
  • Creation of a fictional library. I think this makes the tutorial a bit more playful and interesting, instead of using filler/dummy text all the time. I took this idea from this Sphinx tutorial by @brandon-rhodes (and left some Easter eggs for the attentive reader)

Looking forward to hearing Sphinx maintainers' thoughts on this!

cc @ericholscher

Relates

doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
@tk0miya tk0miya added type:docs type:proposal a feature suggestion labels May 29, 2021
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.

Thank you for your contribution!
LGTM with nits!

doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/contents.rst Show resolved Hide resolved
@astrojuanlu
Copy link
Contributor Author

Thanks a lot for the prompt review @tk0miya , and for your comment @jakobandersen ! I addressed most of the comments. Happy to lay the first stone of this!

@tk0miya
Copy link
Member

tk0miya commented May 29, 2021

Thank you for your update. LGTM again.
Do you have a plan to update this more? I need to know what is a good time for merging this.

I'll wait for a while for reviews from other contributors.

@astrojuanlu
Copy link
Contributor Author

Do you have a plan to update this more?

Only the installation.rst part, that'll be quick. Apart from that, I consider this complete.

Copy link
Contributor

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

This is an excellent start. There are bunch of subtle things that people who write documentation will appreciate, including the use of the second person voice. That is difficult to pull off well, and I congratulate you.

I have a few points for your consideration.

  1. Please do not use semantic line wrapping. There are strong advocates for this style, but it has several problems, including:

    • Nobody writes real prose like that.
    • It is alien to people who do not write English as their first language.
    • It hurts readability of the source, which ends up looking like lit-magzine prose-poetry.
    • It has no effect on rendered output, which is where the value should be.
    • Adds considerable difficulty to create translations.
    • It is inconsistent with the current arbitrarily and historically chosen 79-character line wrapping (which itself is also not helpful, IMO)

    If any new style should be adopted, I would strongly advocate for the simple to understand "one line per sentence" style.

    • Easy to remember.
    • Easy to translate.
    • Easy to read diffs.
    • No more rewrapping entire paragraphs when making a single typo correction.
    • Code editors have supported soft line wraps for decades.
  2. Use 4 spaces for indenting code-blocks, list items, and other things. For an explanation, please see https://stackoverflow.com/questions/48311159/is-3-space-indentation-required-in-rest/48313531#48313531 and the "bonus tip".

  3. Please leave this open to allow feedback. This is Memorial Day Weekend in the United States, and some folks extend a 3-day weekend into a 9-day holiday.

doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/usage/installation.rst Outdated Show resolved Hide resolved
doc/usage/installation.rst Outdated Show resolved Hide resolved
doc/usage/installation.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
doc/tutorial/index.rst Outdated Show resolved Hide resolved
@tk0miya
Copy link
Member

tk0miya commented May 30, 2021

Please do not use semantic line wrapping. There are strong advocates for this style, but it has several problems, including:

I don't have a big opinion about this. But it's like a bikeshed discussion. I believe it must not be a blocker of improvement. So this issue is not good to discuss. How about below?

  • Until the new rule is determined, follow the current rule (folded by 80 chars)
  • Discuss the new rule in the new issue (if somebody would like to do. I'm not interested)

Use 4 spaces for indenting code-blocks, list items, and other things. For an explanation, please see https://stackoverflow.com/questions/48311159/is-3-space-indentation-required-in-rest/48313531#48313531 and the "bonus tip".

This is also.

At present, almost of our document is rewritten with this (implicit) rule:

  • 2 spaces for bullet lists
  • 3 spaces for directive contents
  • 4 spaces for other indentation

Please leave this open to allow feedback. This is Memorial Day Weekend in the United States, and some folks extend a 3-day weekend into a 9-day holiday.

Okay, I'll keep this open until the end of the next weekend.

@astrojuanlu
Copy link
Contributor Author

Thanks for the super detailed feedback @stevepiercy , really appreciate it! Yes, I am not a native English speaker myself (as you might have guessed already) and I spent a lot of time getting used to using the second person in tutorials, I'm happy the expert eye spotted it :)

About semantic linefeeds, you raise valid criticism and I'd like to discuss it further. Therefore, as @tk0miya said, I'll rewrap all text in 80 columns and open an issue about style later on (after I'm done addressing your comments - to avoid messing everything up).

About indentation, I wanted to be consistent with the rest of Sphinx, although I might have made some mistakes. I prefer to stick to what @tk0miya said.

Going over your comments now.

astrojuanlu and others added 2 commits May 31, 2021 16:26
Co-authored-by: Steve Piercy <web@stevepiercy.com>
@tk0miya
Copy link
Member

tk0miya commented Jun 6, 2021

I think the docslint failure is unrelated to this PR.

I fixed them it in #9303. Could you merge the HEAD of 4.x into this branch?

@tk0miya
Copy link
Member

tk0miya commented Jun 6, 2021

Okay, I'll keep this open until the end of the next weekend.

It seems all discussions are resolved (I know we still have a discussion for doc-styling). So I'd like to merge this if nobody objects within a day.

@stevepiercy Do you have comment for the updates? Please let us know if you find another point.

@stevepiercy
Copy link
Contributor

@tk0miya I'll defer style discussion to other issues for the sake of merging this PR. It makes sense to continue existing practices, and thank you for clarifying them where they exist.

I have only one objection, specifically about the name of the virtual environment. I am very strongly in favor of keeping it short, simple, and pronounceable because I would use this material to conduct trainings. Two fewer characters to type and syllables to speak, especially when repeated throughout the docs, would be relief to students and the instructor. Compare "vee-ee-en-vee space dot-vee-ee-en-vee" versus "vee-ee-en-vee space ee-en-vee". If it were just a single usage, I would not care, but it would be used throughout the material.

@astrojuanlu
Copy link
Contributor Author

I fixed them it in #9303. Could you merge the HEAD of 4.x into this branch?

Sure! Do you usually do merge or rebase? (I prefer the latter, but I want to be consistent with what's usually done in Sphinx)

About the name of the virtual environment, my Twitter poll of 32 votes yielded a clear advantage over .venv https://mobile.twitter.com/juanluisback/status/1399375962228224005 I know it's by no means a big sample, but it matches my observations of (a) keeping it hidden and (b) avoiding overlap with direnv (Brett Cannon agrees https://mobile.twitter.com/brettsky/status/1399439424929230850)

@astrojuanlu
Copy link
Contributor Author

(I went ahead and merged)

@stevepiercy
Copy link
Contributor

I have doubts a student of this tutorial uses direnv, but if they do, they are most likely not a Sphinx newbie, they know what they are doing with virtual environments, and they will have their own opinions and preferences for them.

I am advocating for the audience of Sphinx newbies and the trainers who will teach them using this material. I feel very strongly about this. I really don't want to spend time explaining to Python newbies the difference between the command "venv" and the name of a virtual environment that happens to be "dot-venv". It's hard enough explaining what is a Python virtual environment in the first place.

@astrojuanlu
Copy link
Contributor Author

I don't have anything meaningful to add here, already voiced my technical arguments. I am also a Python instructor and I don't see much difficulty, but this is subjective. Don't want to die on this particular hill and I am not a fan of bikeshedding, so I defer to @tk0miya to make the final call, and I'll be happy to make changes if needed.

@tk0miya
Copy link
Member

tk0miya commented Jun 6, 2021

I don't have any opinion for the name of virtualenv directory.

In my experience, it's difficult to teach what virtualenv is for non python developers. So, personally, it's too much to use it on the tutorials. I know, my opinion is a bit extreme. But the installation of python itself and its packages is too complicate to describe (conda, pyenv, virtualenv, etc. etc. etc.). So I'd not like to touch the topic...

Copy link
Contributor

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

This feels like a great first iteration. I think this can be merged as is now! :)

@pradyunsg
Copy link
Contributor

pradyunsg commented Jun 11, 2021

So, personally, it's too much to use it on the tutorials. I know, my opinion is a bit extreme. But the installation of python itself and its packages is too complicate to describe (conda, pyenv, virtualenv, etc. etc. etc.). So I'd not like to touch the topic...

puts on documentation author hat
puts on pip maintainer hat
puts on past maintainer of virtualenv hat

And... as you've probably guessed, I disagree that we should avoid it mentioning virtual environments here.

The goal of a tutorial is NOT to provide the user with all their options, but rather to give them a step-by-step guided learning journey. I think we should definitely recommend using virtual environments, especially in tutorial style content. One of the big reasons to introduce them early is to protect beginners from the problems of messing up their OS, especially on Linux. Avoiding mentioning virtual enviroments risks breakage due to inexperienced users breaking their operating systems.

FWIW, I am planning on making a round of improvements to packaging.python.org at some point in the near future to make it easier to defer virtualenv-specific stuff to there, but I think what this tutorial is doing right now, is basically perfect -- not going into too much detail, but still doing the right thing.

@astrojuanlu
Copy link
Contributor Author

@tk0miya Any chance to get this merged? In any case, I already started working on the second part, and I hope to send another PR this week.

@tk0miya
Copy link
Member

tk0miya commented Jun 13, 2021

@pradyunsg Thank you for the comment. Okay, let's go with .venv. Merging now.

@tk0miya tk0miya merged commit bedbb8c into sphinx-doc:4.x Jun 13, 2021
@tk0miya
Copy link
Member

tk0miya commented Jun 13, 2021

Merged. @astrojuanlu Good work!

And, thank you for all :-)

@astrojuanlu astrojuanlu deleted the new-tutorial branch June 13, 2021 15:20
@astrojuanlu
Copy link
Contributor Author

Thanks to you! On to the next part 💪🏽

@jfbu
Copy link
Contributor

jfbu commented Jun 13, 2021

I encounter a

! Package inputenc Error: Unicode character ˈ (U+02C8)
(inputenc)                not set up for use with LaTeX.

when building the sphinx.pdf after this. I wll investigate...

@jfbu
Copy link
Contributor

jfbu commented Jun 13, 2021

@astrojuanlu I am not familiar with phonetics. Would it be ok to replace /luˈmake/ with /lu'make/ ?
Else I can add to our conf.py for latex pdf build a \DeclareUnicodeCharacter{02C8}{\textquotesingle} but perhaps there is a better way involving loading a special font for characters used in phonetics?

@jfbu
Copy link
Contributor

jfbu commented Jun 13, 2021

Is this rewrap intended:

.. code-block:: text

    docs ├── build ├── make.bat ├── Makefile └── source ├── conf.py ├──
    index.rst ├── _static └── _templates

? I think it crept in at some point unintended...

@astrojuanlu
Copy link
Contributor Author

Oh right, that was unintended... I thought I had fixed it locally, my bad. Thanks @jfbu

I am not an expert either, but I don't think this warrants an extra unicode character, probably your suggested change is good enough. Will fix both things soon.

@astrojuanlu
Copy link
Contributor Author

#9339

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 15, 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

7 participants