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

Add existing guidelines to qiskit docs guidelines #113

Conversation

HuangJunye
Copy link
Contributor

@HuangJunye HuangJunye commented Oct 20, 2022

Summary

We have written documentation guidelines in Box notes. This PR migrate those guidelines into .rst format within the new Qiskit Docs Guidelines sphinx project created by #108.

  • Add tutorials guidelines
  • Add how-to guide guidelines
  • Add API reference guidelines
  • Add explanation guidelines
  • Simply internal cross-refernecing in toctree (see details below)
  • Rename explanations to explanation

Fixes #81, Fixes #82, Fixes #83, Fixes #84.

This is a continuation of #100 which was closed because of branch renaming.

Details

This PR also simplifies the internal cross-referencing in toctree by putting the name of files instead of names with links. For example, previously we use:

.. toctree::

   Getting Started <getting_started>

now we use:

.. toctree::

   getting_started

The linked text is extracted from the title of getting_started file which is the same Getting Started. But using this new way we simply the cross referencing and avoid extra work on making sure the text Getting Started inside toctree matches the title of the linked page.

Details

@HuangJunye HuangJunye changed the base branch from main to feature/docs-guidelines October 20, 2022 10:18
@HuangJunye HuangJunye mentioned this pull request Oct 20, 2022
4 tasks
HuangJunye and others added 6 commits October 20, 2022 12:49
Co-Authored-By: Guillermo-Mijares-Vilarino <106545082+Guillermo-Mijares-Vilarino@users.noreply.github.com>
Co-authored-by: Guillermo Mijares-Vilarino <Guillermo-Mijares-Vilarino@users.noreply.github.com>
@HuangJunye HuangJunye force-pushed the feature/docs-guidelines/add-existing-guidelines branch from 7562ffe to 5fbc04b Compare November 8, 2022 09:56
HuangJunye and others added 6 commits November 8, 2022 11:14
Co-authored-by: Guillermo Mijares-Vilarino <Guillermo-Mijares-Vilarino@users.noreply.github.com>
Co-authored-by: Guillermo Mijares-Vilarino <Guillermo-Mijares-Vilarino@users.noreply.github.com>
@HuangJunye HuangJunye force-pushed the feature/docs-guidelines/add-existing-guidelines branch from ae8b707 to 70ea2ea Compare November 8, 2022 10:57
@HuangJunye HuangJunye marked this pull request as ready for review November 8, 2022 11:15
Comment on lines +20 to +25
getting_started
tutorials/index
how_to/index
apidocs/index
explanation/index
release_notes
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code is cleaner this way! Nice detail.

Not sure about changing "explanations" to "explanation". I saw explanations as countable in the same way as tutorials, that is, each file is "an explanation".

However, now that I check the Diataxis page, explanation is used as uncountable there.

The word "explanation" can be used as both countable and uncountable so I guess it's a matter of choosing one and sticking to it.

Comment on lines +11 to +14
Lower the barrier of entry as much as possible. Assume the user does not know much about what is
covered in the tutorial. If we are expecting users to know something, make it explicit and link them
to the needed explanations (See :ref:`tutorial-guidelines-minimum-explanation`) Avoid jargon as much as possible. Provide a minimum
necessary explanation when the jargon is introduced. (See :ref:`tutorial-guidelines-minimum-explanation`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be better to separate a bit more the different points for readability.

Something like:

Lower the barrier of entry as much as possible. Assume the user does not know much about what is
covered in the tutorial.

If we are expecting users to know something, make it explicit and link them
to the needed explanations (See :ref:tutorial-guidelines-minimum-explanation)

Avoid jargon as much as possible. Provide a minimum
necessary explanation when the jargon is introduced. (See :ref:tutorial-guidelines-minimum-explanation)

Copy link
Collaborator

@javabster javabster 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 looking great! I left some comments, mostly just grammar/typos.

Also a couple of general things:

  • I think the numbered headings are too large when they actually get built, maybe you could use a different underline to make sphinx give it a smaller font size?
  • When i build locally there seems to be some weird stuff happening with the toc tree, all your changes seem to be under API Reference, and nothing is showing under How-to-guides etc (screenshots attached)

Screenshot 2022-11-14 at 2 24 56 PM

Screenshot 2022-11-14 at 2 25 06 PM

docs_guidelines/apidocs/api.guidelines.rst Outdated Show resolved Hide resolved
@@ -0,0 +1,57 @@
########################
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be good to add a sentence or 2 at the top of this page to introduce what is going to be covered and when users should use this

Copy link
Collaborator

Choose a reason for hiding this comment

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

For example something like: "This section provides guidelines for writing API References. This could apply to individual class or function pages, as well as module level pages ..."

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would also be good to reference our internal guidelines for docstrings, which follows the google spec i think

docs_guidelines/apidocs/api.guidelines.rst Outdated Show resolved Hide resolved
doc building process complexity. See
`qiskit-terra#7661 <https://github.com/Qiskit/qiskit-terra/issues/7661>`_ for discussions.

1. Try to show as many arguments/options as possible within reason
Copy link
Collaborator

Choose a reason for hiding this comment

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

Show where? do you mean in the examples on the API reference? Or the actual docstring? They need to list all arguments in the docstring otherwise lint will fail. A bit more clarity in this section would be great

Copy link
Collaborator

Choose a reason for hiding this comment

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

This API guidelines are only about code examples. Maybe we should clarify that 😅

docs_guidelines/apidocs/api.guidelines.rst Outdated Show resolved Hide resolved
3.1 Test tutorials regularly
----------------------------

on supported platform and python / Qiskit versions (for example, via CI
Copy link
Collaborator

Choose a reason for hiding this comment

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

this reads like a note, i think it should be a full sentence. Also explain what CI is otherwise we aren't practicing what we preach about jargon haha

Copy link
Collaborator

Choose a reason for hiding this comment

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

These guidelines are supposed to be mainly for the mantainers of a Qiskit package. I think it's fair to assume they know what CI is. Also don't forget that this is not supposed to be a tutorial (so jargon is not so big a problem) but I can add an explanation about CI. What do you think? @javabster @HuangJunye

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah most of the time i think it's fine to assume that, it's just that i was reading this straight after the section when you talk about not using jargon etc. and it just felt a bit hypocritical 😅

Copy link
Collaborator

Choose a reason for hiding this comment

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

i wouldn't block merging on this bc of the acronym, just a suggestion. I do think it should be a full sentence though, and generally i think we should keep the grammar consistent in the page. so either all short note form or all full sentences, not a mix of both

docs_guidelines/apidocs/tutorials.guidelines.rst Outdated Show resolved Hide resolved
docs_guidelines/apidocs/tutorials.guidelines.rst Outdated Show resolved Hide resolved
docs_guidelines/apidocs/tutorials.guidelines.rst Outdated Show resolved Hide resolved
docs_guidelines/apidocs/tutorials.guidelines.rst Outdated Show resolved Hide resolved
@Guillermo-Mijares-Vilarino
Copy link
Collaborator

When i build locally there seems to be some weird stuff happening with the toc tree, all your changes seem to be under API Reference, and nothing is showing under How-to-guides etc (screenshots attached)

This is done on purpose. The guidelines, examples and templates themselves are our "API Reference" but each one is about tutorials, how-tos, reference or explanation, so you can see the how-to section as "the part of the docs/guidelines API reference that is about creating how-tos".

It's a bit confusing but it makes sense

@javabster
Copy link
Collaborator

When i build locally there seems to be some weird stuff happening with the toc tree, all your changes seem to be under API Reference, and nothing is showing under How-to-guides etc (screenshots attached)

This is done on purpose. The guidelines, examples and templates themselves are our "API Reference" but each one is about tutorials, how-tos, reference or explanation, so you can see the how-to section as "the part of the docs/guidelines API reference that is about creating how-tos".

It's a bit confusing but it makes sense

I don't think this makes sense tbh 😅 I think it would be better for readers to have each guideline live in the page associated with what the guidelines are about. So tutorial guidelines under the tutorials section, explanation guidelines under the explanation section etc.. Also our docs aren't an API so it's not correct for us to say that the guidelines form the API Reference, there is no API that we are referencing

@Guillermo-Mijares-Vilarino
Copy link
Collaborator

I don't think this makes sense tbh 😅 I think it would be better for readers to have each guideline live in the page associated with what the guidelines are about. So tutorial guidelines under the tutorials section, explanation guidelines under the explanation section etc.. Also our docs aren't an API so it's not correct for us to say that the guidelines form the API Reference, there is no API that we are referencing

Yeah we talked about the lack of actual API and agreed to rename "API Reference" to "Reference". The solution to this confusion (that we have in mind right now) is to add links from say the how-to page (with the actual how-to guides) to the Reference about how-to guides. Something like:

This section covers how-to guides for creating documentation, if you are looking for examples, templates or guidelines about writing how-to guides, check this reference material (link to how-to reference)

I believe putting say the how-to examples, templates and guidelines in the how-to section defeats the purpose of using Diataxis for docs_guidelines, as they are not how-to guides 😅

@javabster
Copy link
Collaborator

Honestly I don't really understand why we are using diataxis so strictly for the docs guidelines themselves 😅 I was assuming this is documentation to teach people how to use the diataxis framework in the context of Qiskit, and I don't think that necessitates implementing diataxis itself. Looking at https://diataxis.fr they keep info about writing tutorials under the tutorials page and info about api reference under the reference page etc etc. they don't put everything under reference and I don't think we should either. The guidelines aren't a piece of software so diataxis in this context just adds confusion for the user for the sake of showcasing diataxis, which I don't think is the best approach. Maybe we can discuss more in our next meeting, it's hard for me to get the full picture of your plans for the guidelines from individual PRs :)

HuangJunye and others added 9 commits December 15, 2022 15:33
Co-authored-by: Guillermo-Mijares-Vilarino <106545082+Guillermo-Mijares-Vilarino@users.noreply.github.com>
Co-authored-by: Guillermo-Mijares-Vilarino <106545082+Guillermo-Mijares-Vilarino@users.noreply.github.com>
Co-authored-by: Guillermo-Mijares-Vilarino <106545082+Guillermo-Mijares-Vilarino@users.noreply.github.com>
Co-authored-by: Abby Mitchell <23662430+javabster@users.noreply.github.com>
Co-authored-by: Abby Mitchell <23662430+javabster@users.noreply.github.com>
Co-authored-by: Abby Mitchell <23662430+javabster@users.noreply.github.com>
Co-authored-by: Abby Mitchell <23662430+javabster@users.noreply.github.com>
Co-authored-by: Abby Mitchell <23662430+javabster@users.noreply.github.com>
Co-authored-by: Abby Mitchell <23662430+javabster@users.noreply.github.com>
@HuangJunye
Copy link
Contributor Author

This PR is superseded by #166

@HuangJunye HuangJunye closed this Jan 10, 2023
@HuangJunye HuangJunye deleted the feature/docs-guidelines/add-existing-guidelines branch January 17, 2023 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants