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 return statement to grdclip and grdgradient docstring #1390

Merged
merged 6 commits into from
Jul 22, 2021
Merged

Add return statement to grdclip and grdgradient docstring #1390

merged 6 commits into from
Jul 22, 2021

Conversation

kadatatlukishore
Copy link
Contributor

@kadatatlukishore kadatatlukishore commented Jul 20, 2021

Description of proposed changes

Updated the docstring in grdclip.py to have a return statement.

Fixes #1388

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

@welcome
Copy link

welcome bot commented Jul 20, 2021

💖 Thanks for opening this pull request! 💖

Please make sure you read our contributing guidelines and abide by our code of conduct.

A few things to keep in mind:

  • If you need help writing tests, take a look at the existing ones for inspiration. If you don't know where to start, let us know and we'll walk you through it.
  • All new features should be documented. It helps to write the docstrings for your functions/classes before writing the code. This will help you think about your code design and results in better code.
  • No matter what, we are really grateful that you put in the effort to do this! 🎉

@kadatatlukishore
Copy link
Contributor Author

Hey @meghanrjones , I have just added the docstring please review. Thanks.

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.

Thanks for your pull request, @kadatatlukishore! Nice work! I have made two small suggestions that add one blank line. The first is for separation from the other section of the docstring and the second is required for the list to be rendered properly in sphinx. You can check out the Vercel preview if you want to see the built docs.

pygmt/src/grdclip.py Show resolved Hide resolved
pygmt/src/grdclip.py Show resolved Hide resolved
@kadatatlukishore
Copy link
Contributor Author

Hello @meghanrjones , made the required changes. Preview look fine. Thanks for the suggestions !

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 great!

I'll leave this PR open for up to a day before merging, just in case any of the other maintainers have comments. Thanks again for your contribution!

@maxrjones
Copy link
Member

/format

@maxrjones maxrjones added documentation Improvements or additions to documentation final review call This PR requires final review and approval from a second reviewer labels Jul 20, 2021
@weiji14
Copy link
Member

weiji14 commented Jul 21, 2021

Seems like grdgradient could also use the same treatment 🙂 @kadatatlukishore, if you have time, do you mind copy-and-pasting the same chunk of documentation to

Thanks!

@maxrjones
Copy link
Member

Seems like grdgradient could also use the same treatment 🙂 @kadatatlukishore, if you have time, do you mind copy-and-pasting the same chunk of documentation to

Thanks!

@weiji14 do you want this change in the same or a separate pull request?

@weiji14
Copy link
Member

weiji14 commented Jul 21, 2021

Seems like grdgradient could also use the same treatment slightly_smiling_face @kadatatlukishore, if you have time, do you mind copy-and-pasting the same chunk of documentation to

Thanks!

@weiji14 do you want this change in the same or a separate pull request?

I think it's ok to have it in the same pull request since it is a related change and not too big. But ok with making it into a separate thing if it's a bit troublesome.

@kadatatlukishore
Copy link
Contributor Author

Seems like grdgradient could also use the same treatment slightly_smiling_face @kadatatlukishore, if you have time, do you mind copy-and-pasting the same chunk of documentation to

Thanks!

@weiji14 do you want this change in the same or a separate pull request?

I think it's ok to have it in the same pull request since it is a related change and not too big. But ok with making it into a separate thing if it's a bit troublesome.

I can work on the same pull request. Thanks @weiji14

@maxrjones
Copy link
Member

/format

@kadatatlukishore
Copy link
Contributor Author

Hello @meghanrjones , Should I have to make any changes ?

Copy link
Member

@seisman seisman left a comment

Choose a reason for hiding this comment

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

Have to manually remove these whitespaces in blank lines.

pygmt/src/grdgradient.py Outdated Show resolved Hide resolved
pygmt/src/grdgradient.py Outdated Show resolved Hide resolved
@weiji14 weiji14 changed the title Added return statement in docstring - grdclip.py Add return statement to grdclip and grdgradient docstring Jul 22, 2021
@seisman seisman added this to the 0.5.0 milestone Jul 22, 2021
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Jul 22, 2021
@seisman seisman merged commit 945bbc5 into GenericMappingTools:master Jul 22, 2021
@welcome
Copy link

welcome bot commented Jul 22, 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.

@maxrjones
Copy link
Member

Great work @kadatatlukishore and thanks again for addressing this issue! 🎉

@seisman seisman modified the milestones: 0.5.0, 0.4.1 Jul 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add returns section to grdclip docstring
6 participants