Add projection and region to grdview docstring#1295
Conversation
weiji14
left a comment
There was a problem hiding this comment.
Oh wait, actually, we're missing the region (R) docstring as well for this!
Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
weiji14
left a comment
There was a problem hiding this comment.
So region (-R) isn't actually a required argument for grdview according to https://docs.generic-mapping-tools.org/6.2.0rc2/grdview.html, but using {R} in the docstring here says that it is:
Wondering if we should edit the wording slightly at
pygmt/pygmt/helpers/decorators.py
Lines 17 to 21 in 01e2feb
Can open a separate issue/PR for this.
I think the current wording is accurate for most plotting modules, so maybe I can just not use the replacement text for |
|
Is the latest version OK with you, @weiji14? |
Yep, looks good! I like that it explicitly specifies zmin/zmax too. Remember to change the PR title to include 'region' before merging. |
|
Sounds good. One more question - when the checks fail for reasons clearly unrelated to the PR (like now), do you prefer for me to re-run all checks or admin merge the PR? |
I would just admin merge in this case to conserve CI resources. |
Description of proposed changes
The
projectionparameter is included in the list of aliases but not the docstring for grdview. This PR adds the projection description to the docstring.Fixes #
Reminders
make formatandmake checkto make sure the code follows the style guide.doc/api/index.rst.Slash Commands
You can write slash commands (
/command) in the first line of a comment to performspecific operations. Supported slash commands are:
/format: automatically format and lint the code/test-gmt-dev: run full tests on the latest GMT development version