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

Document how to contribute a gallery example. #4857

Merged
merged 8 commits into from
Jul 28, 2020

Conversation

mkcor
Copy link
Member

@mkcor mkcor commented Jul 22, 2020

Description

We identified the need for these instructions while reviewing and reading PR reviews.

Checklist

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.

CONTRIBUTING.txt Outdated
~~~~~~~

If you are contributing an example to the
`gallery <https://scikit-image.org/docs/dev/auto_examples/>`_ (or editing an
Copy link
Member Author

Choose a reason for hiding this comment

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

Should this be an internal link (ref?) instead? There is another link to a https://scikit-image.org/ page elsewhere (line 83), so I left it this way...

Copy link
Member

Choose a reason for hiding this comment

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

I think a ref is better, good suggestion! Anyone else @scikit-image/core got an opinion?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done as part of bdfbbd2.

Then I thought I might as well change the other https://scikit-image.org/ link: 0fd5eb9.

Also, from https://thomas-cokelaer.info/tutorials/sphinx/rest_syntax.html, it seems that the link text at

:ref:`Coordinate conventions <numpy-images-coordinate-conventions>`

is unnecessary, 'Coordinate conventions' being the title following the numpy-images-coordinate-conventions label (file doc/source/user_guide/numpy_images.rst).

CONTRIBUTING.txt Outdated
existing one), write in
`reStructuredText <https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html>`_.
Indeed, the gallery is a
`Sphinx-Gallery <https://sphinx-gallery.github.io/stable/index.html>`_ one.
Copy link
Member Author

Choose a reason for hiding this comment

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

"one" might be unnecessary

Copy link
Member

Choose a reason for hiding this comment

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

Agreed! 😂

Copy link
Member

Choose a reason for hiding this comment

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

The gallery is created using Sphinx-Gallery <https://sphinx-gallery.github.io>_. Refer to that page for complete instructions on syntax and usage.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for this better wording! Included in bdfbbd2. I don't love my first sentence in the new [sub * (n + 1)]section ('The above-mentioned command includes the installation of Sphinx-Gallery.') but I thought repeating

pip install -r requirements/docs.txt

wouldn't be great either...

Copy link
Member

@jni jni left a comment

Choose a reason for hiding this comment

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

@mkcor thank you for tackling this! I agree with both of your self-suggestions 😂 and have generated a couple of new ones. Thank you!

CONTRIBUTING.txt Outdated Show resolved Hide resolved
CONTRIBUTING.txt Outdated
existing one), write in
`reStructuredText <https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html>`_.
Indeed, the gallery is a
`Sphinx-Gallery <https://sphinx-gallery.github.io/stable/index.html>`_ one.
Copy link
Member

Choose a reason for hiding this comment

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

The gallery is created using Sphinx-Gallery <https://sphinx-gallery.github.io>_. Refer to that page for complete instructions on syntax and usage.

Copy link
Member

@stefanv stefanv left a comment

Choose a reason for hiding this comment

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

Thanks for this, Marianne!

CONTRIBUTING.txt Outdated
Indeed, the gallery is a
`Sphinx-Gallery <https://sphinx-gallery.github.io/stable/index.html>`_ one.

Refer to the above to build (all) the docs, and check how your edits render at
Copy link
Member

Choose a reason for hiding this comment

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

Maybe mention the browser here, otherwise newcomers might not know how to check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, realized that when replying: #4851 (comment)
Done bdfbbd2.

CONTRIBUTING.txt Outdated
``scikit-image/doc/build/html/auto_examples/`` (navigate to the file you have
added or changed).

When adding an example, visit also
Copy link
Member

Choose a reason for hiding this comment

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

also visit

Copy link
Member Author

Choose a reason for hiding this comment

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

I find there is a slight difference in meanings, but I'm not sure: I wrote 'visit also index.html' because the contributor was just told to visit another page (visit A, visit also B); 'also visit' sounds like the previous instruction would entail an action different from visiting a web page (e.g., run make html, also visit the resulting web pages).

CONTRIBUTING.txt Outdated Show resolved Hide resolved
@alexdesiqueira alexdesiqueira added the 📄 type: Documentation Updates, fixes and additions to documentation label Jul 25, 2020
@mkcor
Copy link
Member Author

mkcor commented Jul 26, 2020

Ready for the next round of reviews, thanks!

Copy link
Member

@alexdesiqueira alexdesiqueira left a comment

Choose a reason for hiding this comment

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

Great addition! Thanks @mkcor!

Copy link
Member

@jni jni left a comment

Choose a reason for hiding this comment

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

Thanks! I've made some very minor suggestions.

CONTRIBUTING.txt Outdated
@@ -222,13 +221,13 @@ Guidelines
* All code should have tests (see `test coverage`_ below for more details).
* All code should be documented, to the same
`standard <https://numpydoc.readthedocs.io/en/latest/format.html#docstring-standard>`_ as NumPy and SciPy.
* For new functionality, always add an example to the gallery.
* For new functionality, always add an example to the gallery (see
sphinx_gallery_ below for more details).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
sphinx_gallery_ below for more details).
:ref:`_sphinx_gallery` below for more details).

?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should be

:ref:`sphinx_gallery`

then. I thought I would try the 'first method' (according to https://thomas-cokelaer.info/tutorials/sphinx/rest_syntax.html#explicit-links) but I'm happy to use :ref: everywhere (might be more standard, reading https://sublime-and-sphinx-guide.readthedocs.io/en/latest/references.html).

Copy link
Member Author

Choose a reason for hiding this comment

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

Done 69e41b5.

CONTRIBUTING.txt Outdated Show resolved Hide resolved
CONTRIBUTING.txt Outdated Show resolved Hide resolved
mkcor and others added 3 commits July 28, 2020 10:17
@mkcor
Copy link
Member Author

mkcor commented Jul 28, 2020

@jni thanks for your final review! I have included your suggestions.

CONTRIBUTING.txt Outdated Show resolved Hide resolved
@emmanuelle
Copy link
Member

The Appveyor failure seems to be independent of this PR, so I'm merging. Thank you @mkcor!

@emmanuelle emmanuelle merged commit ebb53a6 into scikit-image:master Jul 28, 2020
@mkcor mkcor deleted the document-contrib-docs branch July 28, 2020 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📄 type: Documentation Updates, fixes and additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants