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.

Sign up for GitHub

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

Jump to bottom

Update documentation guide on glossary entries and where to put formulae #1264

Merged
merged 9 commits into from
Sep 21, 2021
Merged

Update documentation guide on glossary entries and where to put formulae #1264

merged 9 commits into from
Sep 21, 2021

Conversation

namurphy
Copy link
Member

@namurphy namurphy commented Sep 17, 2021 •
edited
Loading

Following up on #935 and #1156, this PR adds a description of where formulae and extended mathematical stuff should be put in docstrings. Following up on #1149, this discusses how to use glossary terms.

This has a limited enough scope that it'd be helpful to get this into 0.7.0.

@github-actions github-actions bot added the docs PlasmaPy Docs at http://docs.plasmapy.org label Sep 17, 2021
@codecov
Copy link

codecov bot commented Sep 17, 2021 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (main@21d7417). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1264   +/-   ##
=======================================
  Coverage        ?   97.06%           
=======================================
  Files           ?       73           
  Lines           ?     7093           
  Branches        ?        0           
=======================================
  Hits            ?     6885           
  Misses          ?      208           
  Partials        ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21d7417...28903cf. Read the comment docs.

@namurphy namurphy changed the title Describe conventions for putting formulae in docstrings Update documentation guide on glossary entries and where to put formulae Sep 17, 2021
@namurphy namurphy added this to the 0.7.0 milestone Sep 17, 2021
@namurphy namurphy requested a review from a team September 17, 2021 14:26
@pheuer
Copy link
Member

pheuer commented Sep 17, 2021 •
edited
Loading

LGTM: My one observation is that, as a new-ish python developer, I don't know what it means for :term: to be a "role". That itself may need some defining. You could potentially just remove that sentence and let the example in the second sentence speak for itself.

@namurphy
Update documentation guide
16f5e05
@namurphy
Update documentation guide
4ad533e 
- Use quasi-horrible substitutions to combine a hyperlink with :file: role
- Link to Sphinx's glossary for role & directive
- Add admonition about Sphinx rabbit holes
- Additional changes here and there
@namurphy namurphy added the status: ready for review PRs that are ready for code review label Sep 21, 2021
@namurphy namurphy requested a review from pheuer September 21, 2021 22:03
@namurphy
Copy link
Member Author

@pheuer — might you have a chance to look at this again? Thanks!

pheuer
pheuer approved these changes Sep 21, 2021
View reviewed changes
Copy link
Member

@pheuer pheuer left a comment

Choose a reason for hiding this comment

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

Looks good!

General question: if |role| links back to the Sphinx glossary, what happens if we re-define role in the plasmapy glossary?

@namurphy
Copy link
Member Author

General question: if |role| links back to the Sphinx glossary, what happens if we re-define role in the plasmapy glossary?

Good point! I updated this to say that the intersphinx terms are used if they haven't already been defined in PlasmaPy's glossary. And thanks!

@namurphy namurphy enabled auto-merge (squash) September 21, 2021 23:43
@namurphy namurphy merged commit 4a353f2 into PlasmaPy:main Sep 21, 2021
Tlord18 pushed a commit to Tlord18/PlasmaPy that referenced this pull request Oct 6, 2021
@namurphy @Timelord18
Update documentation guide on glossary entries and where to put formu…
c6d1e3d 
…lae (PlasmaPy#1264)

* Update documentation guide

* Ensure that the glossary is sorted alphabetically

* Discuss glossary entries in the documentation guide

* Add changelog entry

* Slight wording change

* Fix link

* Update documentation guide

* Update documentation guide

- Use quasi-horrible substitutions to combine a hyperlink with :file: role
- Link to Sphinx's glossary for role & directive
- Add admonition about Sphinx rabbit holes
- Additional changes here and there

* Clarify about priority of glossaries with :term:
Tlord18 pushed a commit to Tlord18/PlasmaPy that referenced this pull request Oct 6, 2021
@namurphy @Timelord18
Update documentation guide on glossary entries and where to put formu…
0cf1930 
…lae (PlasmaPy#1264)

* Update documentation guide

* Ensure that the glossary is sorted alphabetically

* Discuss glossary entries in the documentation guide

* Add changelog entry

* Slight wording change

* Fix link

* Update documentation guide

* Update documentation guide

- Use quasi-horrible substitutions to combine a hyperlink with :file: role
- Link to Sphinx's glossary for role & directive
- Add admonition about Sphinx rabbit holes
- Additional changes here and there

* Clarify about priority of glossaries with :term:
@namurphy namurphy deleted the doc-guide-eqs branch March 16, 2022 15:47
@namurphy namurphy removed the status: ready for review PRs that are ready for code review label Aug 15, 2022
@namurphy namurphy mentioned this pull request Aug 18, 2022
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pheuer pheuer pheuer approved these changes

Assignees
No one assigned
Labels
contributor guide docs PlasmaPy Docs at http://docs.plasmapy.org
Projects
None yet
Milestone
0.7.0
Development

Successfully merging this pull request may close these issues.
2 participants
@namurphy @pheuer