Wrap grdfilter

[carocamargo]

Description of proposed changes

This PR wraps grdfilter.

• Module documentation: https://docs.generic-mapping-tools.org/dev/grdfilter.html
• Julia wrapper: https://www.generic-mapping-tools.org/GMT.jl/latest/#GMT.grdfilter

Fixes #610.

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.

[weiji14]

Alright, onwards with the review, continuing from #612! This may take a lot of rounds of back and forth (similar to peer review, but a lot more fast paced and interactive since it's on Github). I've made some "suggested changes" to the documentation as a start, which you're welcome to "Commit suggestion" directly, or pool several together using "Add suggestion to batch" before committing. See also https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/incorporating-feedback-in-your-pull-request#applying-a-suggested-change.

Of course, you can also edit the suggestions and do it yourself locally, no harm with that. We'll make more fine-grained suggestions later on so as not to overwhelm you too many requests/suggestions at once. I must say that grdfilter looks to be a pretty simple function to wrap, so hopefully it shouldn't take too long 🤞

P.S. You should be able to preview the live documentation at https://pygmt-git-fork-carocamargo-newfeature.gmt.vercel.app once you add grdfilter to the doc/api/index.rst file.

[carocamargo]

Alright, onwards with the review, continuing from #612! This may take a lot of rounds of back and forth (similar to peer review, but a lot more fast paced and interactive since it's on Github). I've made some "suggested changes" to the documentation as a start, which you're welcome to "Commit suggestion" directly, or pool several together using "Add suggestion to batch" before committing. See also https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/incorporating-feedback-in-your-pull-request#applying-a-suggested-change.

Of course, you can also edit the suggestions and do it yourself locally, no harm with that. We'll make more fine-grained suggestions later on so as not to overwhelm you too many requests/suggestions at once. I must say that grdfilter looks to be a pretty simple function to wrap, so hopefully it shouldn't take too long 🤞

P.S. You should be able to preview the live documentation at https://pygmt-git-fork-carocamargo-newfeature.gmt.vercel.app once you add grdfilter to the doc/api/index.rst file.

Ok.. this will sound silly, but then how can I edit the file after I have done the PR? Because if I change it locally, and push it to my branch, then I need to do a new PR to the pygmt, no? Is there a way to add files to this PR?

[seisman]

If you're using git command line, you can run

# checkout your local branch
git checkout newfeature
# update your localbranch
git pull 

[weiji14]

Ok.. this will sound silly, but ...

There are no silly questions :), let's take a step back then. I recommend following https://www.digitalocean.com/community/tutorials/hacktoberfest-how-to-submit-your-first-pull-request-on-github to get a step by step introduction on how things work. Let us know if you need any clarification with specific things.

then how can I edit the file after I have done the PR? Because if I change it locally, and push it to my branch, then I need to do a new PR to the pygmt, no? Is there a way to add files to this PR?

Simplest way to edit the file would be to do it on the Github UI (i.e. at https://github.com/carocamargo/pygmt/edit/newfeature/doc/api/index.rst). You can change branches on Github (to 'newfeature' in this case) and navigate to the file you want to change:

The edit will take place in the same PR, so you don't need to submit a new one.

If you want to add a file for this Pull Request (e.g. a gallery example file), there should be an "Add File" button on the top right to do that too:

Either way (editing/adding files) can also be accomplished on the command line (worth learning when you have time). Just to follow up on @seisman's comment, the steps would be:

cd ~/path/to/pygmt
# checkout your local branch
git checkout newfeature
# update your localbranch
git pull 

# edit/change/add files in your text/code editor

git add doc/api/index.rst  # stage your changed file for commit
git status  # see what you've staged for commiting
git commit -m "Changed doc/api/index.rst to add grdfilter"
git log -2  # see last two commits to double check
git push  # push the local committed changes up to the remote branch on Github

[carocamargo]

< In terms of unit tests, there is a doctest example that might be sufficient for now, but we could perhaps start a test_grdfilter.py file to have more comprehensive tests in place. Happy to do this in a separate PR, if you'd like to keep this one manageable and focused on the raw grdfilter implementation only.>

Think that it's better in another PR, just to not make it so messy. I'll look at the other test files to see what it should include.

[weiji14]

Think that it's better in another PR, just to not make it so messy. I'll look at the other test files to see what it should include.

Sounds good. You can look at https://github.com/GenericMappingTools/pygmt/blob/v0.2.0/pygmt/tests/test_grdcut.py (or any other one) as an example. It's more about testing different file/data input/output types, rather than the grdfilter function itself, i.e. the PyGMT/Python specific stuff rather than the GMT grdfilter function. We can discuss this more when that PR is opened.

[seisman]

@carocamargo This PR is almost ready to merge. Could you please format the codes by running make format?

You may need to update your local branch first, as I made a few changes to your remote branch yesterday (applied the @weiji14's suggestion and merged the master branch).

[seisman]

Ping @carocamargo again to help format the codes, so that we can merge it into master branch.

[carocamargo]

@carocamargo This PR is almost ready to merge. Could you please format the codes by running make format?

You may need to update your local branch first, as I made a few changes to your remote branch yesterday (applied the @weiji14's suggestion and merged the master branch).

Sorry @seisman , but what do you mean by make format? To format the code using Black?

I tried, running, and got this:

(base) Carolinas-MacBook-Pro:pygmt ccamargo$ make format gridops.py 
make: *** No rule to make target `format'.  Stop.

[seisman]

Run make format in the root directory of the pygmt project, not in the pygmt/pygmt subdirectory.

[seisman]

/format

[seisman]

@carocamargo FYI, with PR #646 merged, now it's possible to lint your code automatically by adding /format at the first line of any comments, like #616 (comment).

@weiji14 The "/format" slash command works well.

[seisman]

This PR looks good to me. Should we merge into before v0.2.1?

[weiji14]

Yep, this initial implementation looks good to me! I'll approve and merge it now.

[welcome]

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

[weiji14]

Cool, thanks for giving this a go @carocamargo! Sorry that it took a while. You should be able to use grdfilter directly now via pip install https://github.com/GenericMappingTools/pygmt/archive/master.zip as per https://www.pygmt.org/dev/install.html#installing-pygmt.

As mentioned in #616 (comment), we should add some more tests for this next (in a separate PR), and you're welcome to add a gallery example too if you have time! Will try and sort out your grd2xyz PR at #636 next too.