Skip to content

Add return statement to grdclip and grdgradient docstring#1390

Merged
seisman merged 6 commits intoGenericMappingTools:masterfrom
K-OS-Dev:update-docstring
Jul 22, 2021
Merged

Add return statement to grdclip and grdgradient docstring#1390
seisman merged 6 commits intoGenericMappingTools:masterfrom
K-OS-Dev:update-docstring

Conversation

@K-OS-Dev
Copy link
Contributor

@K-OS-Dev K-OS-Dev commented Jul 20, 2021 •
edited by weiji14
Loading

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

@K-OS-Dev
Copy link
Contributor Author

K-OS-Dev commented Jul 20, 2021

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

maxrjones
maxrjones reviewed Jul 20, 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.

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.

@K-OS-Dev
Copy link
Contributor Author

K-OS-Dev commented Jul 20, 2021

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

maxrjones
maxrjones approved these changes Jul 20, 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 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

maxrjones commented Jul 20, 2021

/format

@K-OS-Dev K-OS-Dev mentioned this pull request Jul 20, 2021
Closed
@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

pygmt/pygmt/src/grdgradient.py

Line 94 in 9b60f5c

{n}

Thanks!

@maxrjones
Copy link
Member

maxrjones 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

pygmt/pygmt/src/grdgradient.py

Line 94 in 9b60f5c

{n}

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

pygmt/pygmt/src/grdgradient.py

Line 94 in 9b60f5c

{n}

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.

@K-OS-Dev
Copy link
Contributor Author

K-OS-Dev 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

pygmt/pygmt/src/grdgradient.py

Line 94 in 9b60f5c

{n}

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

maxrjones commented Jul 21, 2021

/format

@K-OS-Dev
Copy link
Contributor Author

K-OS-Dev commented Jul 21, 2021

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

seisman
seisman reviewed Jul 21, 2021
View reviewed 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.

@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

maxrjones commented Jul 22, 2021

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

@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
@K-OS-Dev
Add return statement to grdclip and grdgradient docstring (GenericMap…
993ba6c 
…pingTools#1390)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seisman seisman seisman approved these changes

@maxrjones maxrjones maxrjones approved these changes

@willschlitzer willschlitzer willschlitzer approved these changes

Assignees

No one assigned

Labels

documentation Improvements or additions to documentation

Projects

None yet

Milestone

0.4.1

Development

Successfully merging this pull request may close these issues.

Add returns section to grdclip docstring

6 participants

@K-OS-Dev @maxrjones @weiji14 @seisman @willschlitzer @actions-bot