Skip to content

Wrap grdsample - #1380

Merged
willschlitzer merged 20 commits into
mainfrom
wrap-grdsample
Aug 9, 2021
Merged

Wrap grdsample#1380
willschlitzer merged 20 commits into
mainfrom
wrap-grdsample

Conversation

@willschlitzer

@willschlitzerwillschlitzer commented Jul 8, 2021

Copy link
Copy Markdown
Contributor

This pull requests wraps the GMT module grdsample.

Fixes#1422

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

@willschlitzerwillschlitzer added the feature Brand new feature label Jul 8, 2021
@willschlitzerwillschlitzer added this to the 0.5.0 milestone Jul 8, 2021
@willschlitzerwillschlitzer self-assigned this Jul 8, 2021
@willschlitzerwillschlitzer changed the title WIP: Wrap grdsampleWrap grdsampleJul 11, 2021

@weiji14weiji14 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @willschlitzer! Just a few suggestions regarding the use of common aliases.

Comment threadpygmt/src/grdsample.py Outdated
Comment threadpygmt/src/grdsample.py Outdated
Comment threadpygmt/src/grdsample.py Outdated
Comment threadpygmt/src/grdsample.py
Comment threadpygmt/src/grdsample.py
Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
@willschlitzer
willschlitzer marked this pull request as ready for review July 16, 2021 15:56
Comment threadpygmt/helpers/decorators.py Outdated
Comment threadpygmt/src/grdsample.py Outdated
Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>

@weiji14weiji14 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me. But please hold off on merging until we decide on whether we want a PyGMT v0.4.1 release, or go straight to v0.5.0 as discussed in #1356 (comment)

@weiji14weiji14 added the final review call This PR requires final review and approval from a second reviewer label Aug 8, 2021

@weiji14weiji14 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for your patience @willschlitzer, now that the v0.4.1 tag is behind, we can finally finish this PR (and all the other ones you have lined up). Could you please resolve the conflict on decorators.py and fix one tiny issue I suggested below?

Comment threadpygmt/src/grdsample.py Outdated
Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
@willschlitzer

Copy link
Copy Markdown
ContributorAuthor

@weiji14 I made the changes you suggested

Comment threadpygmt/tests/test_grdsample.py Outdated
Comment threadpygmt/tests/test_grdsample.py Outdated
Comment threadpygmt/tests/test_grdsample.py Outdated
Comment threadpygmt/tests/test_grdsample.py Outdated

@michaelgrundmichaelgrund left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fine to merge after the suggestions by @seisman are implemented!

Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
Comment threadpygmt/src/grdsample.py Outdated
@use_alias(
G="outgrid",
J="projection",
I="increment",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems other modules (e.g., grdfilter) use the alias spacing rather than increment. We should keep the alias consistent, no?

Comment threadpygmt/src/grdsample.py Outdated
Comment threadpygmt/src/grdsample.py Outdated
Comment threadpygmt/src/grdsample.py Outdated
Comment threadpygmt/tests/test_grdsample.py Outdated

@weiji14weiji14 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🚀

@willschlitzer
willschlitzer merged commit 85d78d6 into mainAug 9, 2021
@willschlitzer
willschlitzer deleted the wrap-grdsample branch August 9, 2021 11:10
@seismanseisman removed the final review call This PR requires final review and approval from a second reviewer label Aug 9, 2021
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
*Wrap GMT module grdsample
*Add tests for grdsample
Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
Co-authored-by: Michael Grund <23025878+michaelgrund@users.noreply.github.com>
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

featureBrand new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

grdsample alternative in pygmt

5 participants

@willschlitzer@michaelgrund@maxrjones@seisman@weiji14