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 common alias "verbose" (V) to grdlandmask and savefig #1343

Merged
merged 2 commits into from
Jun 29, 2021

Conversation

sean0921
Copy link
Contributor

@sean0921 sean0921 commented Jun 21, 2021

Description of proposed changes

Add common alias "verbose" (V) to grdlandmask and savefig before #1281 is solved.

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

Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

Hi @sean0921! Thanks for your PR, I was just reading your blog post https://sean0921.github.io/2021/06/21/pygmt-2021-6-feedback/ just now and really appreciate the honest feedback. A few of us seismologists (cc @core-man) are keen on implementing sac (see #967 (comment)) and will look into it in the coming months. You're welcome to help out if you have time, or open up specific feature request issues for modules like sac and project at https://github.com/GenericMappingTools/pygmt/issues/new/choose.

Anyways, back to this PR. Please add a line with {V} to the docstring so that the verbose documentation will show up. It should look similar to this

pygmt/pygmt/src/grdimage.py

Lines 154 to 156 in b2808cf

{R}
{V}
{XY}

pygmt/src/grdlandmask.py Outdated Show resolved Hide resolved
@core-man
Copy link
Member

@sean0921 Thanks for your PR. Your blog post is great. Welcome to contribute to PyGMT and I believe you could make great help for the project.

@seisman seisman added the enhancement Improving an existing feature label Jun 24, 2021
@seisman seisman added this to the 0.5.0 milestone Jun 24, 2021
@sean0921 sean0921 force-pushed the fix/verbose_alias branch from 93d76ea to c69a8f9 Compare June 28, 2021 14:08
@seisman seisman added the final review call This PR requires final review and approval from a second reviewer label Jun 28, 2021
@sean0921 sean0921 force-pushed the fix/verbose_alias branch from c69a8f9 to cf679a1 Compare June 28, 2021 15:04
@sean0921
Copy link
Contributor Author

sean0921 commented Jun 28, 2021

also add {V} docstring to savefig (a wrapper of psconvert?)
also let savefig API documentation show aliases
It seems not work and there's already a tip for readers to look at psconvert page in savefig page

Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

also add {V} docstring to savefig (a wrapper of psconvert?)
also let savefig API documentation show aliases
It seems not work and there's already a tip for readers to look at psconvert page in savefig page

I think it works, the documentation build at https://pygmt-eowmibjw9-gmt.vercel.app/api/generated/pygmt.Figure.psconvert.html shows the 'verbose' parameter documentation.

Copy link
Member

@michaelgrund michaelgrund 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 to me!

@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Jun 29, 2021
@seisman seisman merged commit d90b3fc into GenericMappingTools:master Jun 29, 2021
@welcome
Copy link

welcome bot commented Jun 29, 2021

🎉🎉🎉 Congrats on merging your first pull request and welcome to the team! 🎉🎉🎉

Please open a new pull request to add yourself to the AUTHORS.md file. We hope that this was a good experience for you. Let us know if there is any way that the contributing process could be improved.

@seisman seisman modified the milestones: 0.5.0, 0.4.1 Jul 25, 2021
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants