Skip to content

Apply NIST SI unit convention to some gallery examples#1194

Merged
seisman merged 18 commits intomasterfrom
gallery-improvements
Apr 13, 2021
Merged

Apply NIST SI unit convention to some gallery examples#1194
seisman merged 18 commits intomasterfrom
gallery-improvements

Conversation

@michaelgrund
Copy link
Member

@michaelgrund michaelgrund commented Apr 8, 2021 •
edited
Loading

Description of proposed changes

I went through the gallery examples and found several things that could be improved:

  • use conventions for SI units: spaces between values and units, adjust axes labels
  • replace green box color in inset example by gold (or any other color which looks more modern)
  • formatting of parameters
  • rename vectors.py to vector_styles.py to avoid warning on "Duplicate example file name(s)"
  • tiny text improvements

In CONTRIBUTING.md

  • add comment about the correct usage of SI units

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

@michaelgrund
Improvements for several gallery examples
86e20d9 
I went through the gallery examples and found several things that could be improved:

- Use conventions for SI units
-
@michaelgrund michaelgrund added the documentation Improvements or additions to documentation label Apr 8, 2021
@michaelgrund michaelgrund added this to the 0.4.0 milestone Apr 8, 2021
@maxrjones
Copy link
Member

maxrjones commented Apr 8, 2021

I find this warning when building the docs to be a bit annoying:

WARNING: Duplicate example file name(s) found. Having duplicate file names will break some links. List of files: ['../examples/tutorials/vectors.py']

Do you think the gallery example vectors.py could be renamed to vector_styles.py or something else to fix the warning as part of these general improvements?

@michaelgrund
Copy link
Member Author

michaelgrund commented Apr 8, 2021 •
edited
Loading

I find this warning when building the docs to be a bit annoying:

WARNING: Duplicate example file name(s) found. Having duplicate file names will break some links. List of files: ['../examples/tutorials/vectors.py']

Do you think the gallery example vectors.py could be renamed to vector_styles.py or something else to fix the warning as part of these general improvements?

Of course, it's a good idea @meghanrjones. I think there will no conflicts appear with any other files. Renamed vectors.py to vectorstyles.py, or do you prefer an underscore in between?

@maxrjones
Copy link
Member

maxrjones commented Apr 8, 2021

I think that vectorstyles.py should have an underscore for improved readability and because most other module names include underscores between words (linestyles.py and linefronts.py being the only exceptions that I noticed in /examples/gallery/.

maxrjones
maxrjones approved these changes Apr 9, 2021
View reviewed changes
Copy link
Member

@maxrjones maxrjones 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! I just have one suggestion for the two list items in CONTRIBUTING.md. If you chose to keep your wording, please just add a period to the end of the sentence.

I learned from that link that I have broken some NIST rules in my papers 🤦

@maxrjones maxrjones added the final review call This PR requires final review and approval from a second reviewer label Apr 9, 2021
@michaelgrund
Apply suggestions from code review
13589b5 
Co-authored-by: Meghan Jones <meghanj@hawaii.edu>
@michaelgrund
Copy link
Member Author

michaelgrund commented Apr 9, 2021

Looks good! I just have one suggestion for the two list items in CONTRIBUTING.md. If you chose to keep your wording, please just add a period to the end of the sentence.

I learned from that link that I have broken some NIST rules in my papers 🤦

Unfortunately these rules and others are often broken in scientific publications 😄, sometimes based on the official styling guidelines of indiviudal journals. My favorite is putting the unit in squared brackets (e.g. distance [m]).

seisman
seisman reviewed Apr 9, 2021
View reviewed changes
@weiji14
Copy link
Member

weiji14 commented Apr 10, 2021

Before merging, could you please change the PR title to something more descriptive like "Apply NIST SI unit convention and fix some gallery examples" (or something along those lines). Thanks!

@michaelgrund michaelgrund changed the title Improvements for several gallery examples Apply NIST SI unit convention and fix some gallery examples Apr 10, 2021
@michaelgrund @seisman
Apply suggestions from code review
349a249 
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
core-man
core-man reviewed Apr 12, 2021
View reviewed changes
@core-man
Copy link
Member

core-man commented Apr 12, 2021

Looks great. Just one minor comment above.

@michaelgrund @core-man
Update examples/gallery/lines/linestyles.py
e29fd10 
Co-authored-by: Yao Jiayuan <coreman.seism@gmail.com>
@michaelgrund
Copy link
Member Author

michaelgrund commented Apr 12, 2021

Looks great. Just one minor comment above.

Done! Thanks @core-man.

seisman
seisman reviewed Apr 12, 2021
View reviewed changes
@seisman seisman changed the title Apply NIST SI unit convention and fix some gallery examples Apply NIST SI unit convention to some gallery examples Apr 13, 2021
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Apr 13, 2021
@seisman seisman merged commit 38f4d1f into master Apr 13, 2021
@seisman seisman deleted the gallery-improvements branch April 13, 2021 17:52
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
@michaelgrund @seisman @core-man
Apply NIST SI unit convention to some gallery examples (GenericMappin…
a8578e6 
…gTools#1194)

Co-authored-by: Meghan Jones <meghanj@hawaii.edu>
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
Co-authored-by: Yao Jiayuan <coreman.seism@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maxrjones maxrjones maxrjones approved these changes

@weiji14 weiji14 weiji14 left review comments

@seisman seisman seisman approved these changes

@core-man core-man core-man approved these changes

Assignees

No one assigned

Labels

documentation Improvements or additions to documentation

Projects

None yet

Milestone

0.4.0

Development

Successfully merging this pull request may close these issues.

5 participants

@michaelgrund @maxrjones @weiji14 @core-man @seisman