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

Wrap dimfilter #1492

Merged
merged 30 commits into from
Apr 11, 2022
Merged

Wrap dimfilter #1492

merged 30 commits into from
Apr 11, 2022

Conversation

willschlitzer
Copy link
Contributor

This pull request wraps the function dimfilter.

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

@willschlitzer willschlitzer added the feature Brand new feature label Sep 11, 2021
@willschlitzer willschlitzer added this to the 0.5.0 milestone Sep 11, 2021
@willschlitzer willschlitzer self-assigned this Sep 11, 2021
@willschlitzer willschlitzer changed the title WIP: Wrap dimfilter Wrap dimfilter Sep 12, 2021
@willschlitzer willschlitzer marked this pull request as ready for review September 12, 2021 10:55
@willschlitzer willschlitzer modified the milestones: 0.5.0, 0.6.0 Oct 7, 2021
@willschlitzer willschlitzer requested a review from a team November 19, 2021 07:20
@maxrjones
Copy link
Member

Sorry for keeping you waiting on a review for this module! dimfilter will be a great addition to the PyGMT processing modules 🎉 I am hoping to find time to check out the branch within the next week.

pygmt/src/dimfilter.py Outdated Show resolved Hide resolved
pygmt/src/dimfilter.py Outdated Show resolved Hide resolved
pygmt/src/dimfilter.py Outdated Show resolved Hide resolved
pygmt/src/dimfilter.py Outdated Show resolved Hide resolved
pygmt/src/dimfilter.py Outdated Show resolved Hide resolved
pygmt/src/dimfilter.py Outdated Show resolved Hide resolved
pygmt/src/dimfilter.py Outdated Show resolved Hide resolved
pygmt/src/dimfilter.py Outdated Show resolved Hide resolved
pygmt/src/dimfilter.py Outdated Show resolved Hide resolved
pygmt/tests/test_dimfilter.py Outdated Show resolved Hide resolved
pygmt/src/dimfilter.py Outdated Show resolved Hide resolved
Co-authored-by: Dongdong Tian <[email protected]>
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.

Just some minor docstring comments. The implementation looks quite good already, and we could merge this new feature 'soon' after a final review, but it'll depend on whether we want to do a PyGMT v0.6.1 or jump straight to PyGMT v0.7.0.

I say we wait until the end of March 2022, and if there isn't any major bug that pops up with users using PyGMT v0.6.0, then we'll jump straight to v0.7.0. What do you think?

pygmt/src/dimfilter.py Outdated Show resolved Hide resolved
pygmt/src/dimfilter.py Outdated Show resolved Hide resolved
pygmt/src/dimfilter.py Outdated Show resolved Hide resolved
@willschlitzer
Copy link
Contributor Author

I say we wait until the end of March 2022, and if there isn't any major bug that pops up with users using PyGMT v0.6.0, then we'll jump straight to v0.7.0. What do you think?

That's fine with me; I'm just hoping to get this wrapped up relatively soon since it's been an open for 6 months.

@seisman
Copy link
Member

seisman commented Mar 21, 2022

I say we wait until the end of March 2022, and if there isn't any major bug that pops up with users using PyGMT v0.6.0, then we'll jump straight to v0.7.0. What do you think?

I agree.

@seisman seisman added the final review call This PR requires final review and approval from a second reviewer label Mar 21, 2022
@willschlitzer
Copy link
Contributor Author

@GenericMappingTools/pygmt-maintainers Are we still holding off for v0.6.1 or am I good to merge this?

@seisman
Copy link
Member

seisman commented Apr 1, 2022

We will release v0.6.1 soon #1856.

@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Apr 11, 2022
@seisman seisman merged commit 2304053 into main Apr 11, 2022
@seisman seisman deleted the wrap-dimfilter branch April 11, 2022 14:58
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
Co-authored-by: Meghan Jones <[email protected]>
Co-authored-by: Dongdong Tian <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
@weiji14 weiji14 mentioned this pull request Mar 30, 2023
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Brand new feature
Projects
Status: 4) Gallery|tutorial example
Development

Successfully merging this pull request may close these issues.

4 participants