-
Notifications
You must be signed in to change notification settings - Fork 224
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 blockmode #1456
Wrap blockmode #1456
Conversation
/format |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, @arleaman! I made a couple small formatting suggestions. Apart from that, there are two more steps needed before adding this module to PyGMT:
- Add
blockmode
to/doc/api/index.rst
in line 84. - Add a test for blockmode. I can provide some instructions for how to add a test for this module, if you expect to have time to work with us on finishing up the pull request?
Ping @arleaman to finalize this PR if available. |
Co-authored-by: Meghan Jones <meghanj@alum.mit.edu>
Co-authored-by: Meghan Jones <meghanj@alum.mit.edu>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me except two minor suggestions.
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 Thanks @arleaman, we'll merge this in about 24 hours if there are no other last minute changes required.
Thanks @meghanrjones! I'll be happy to add tests for the module with some additional guidance!
|
I think we still need one test for blockmode to make sure that the function can be called. |
Great, let's add a test! @arleaman, here is some guidance:
|
Thanks for the help @meghanrjones! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Meghan Jones <meghanj@alum.mit.edu>
Co-authored-by: Meghan Jones <meghanj@alum.mit.edu>
Co-authored-by: Meghan Jones <meghanj@alum.mit.edu>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! 🎉
I'll leave this open for a short while in case others have any last minute comments.
The alias of d is nodata in other modules, but was incorrectly set to data in blockmean and blockmode. This PR fixes the problem. blockmode was wrapped in #1456 (after v0.4.1), blockmean was wrapped in #1092 but the wrong alias was added in #1500 (after v0.4.1). Thus, the change won't go into the deprecation cycle.
Co-authored-by: Meghan Jones <meghanj@alum.mit.edu> Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
…ngTools#1563) The alias of d is nodata in other modules, but was incorrectly set to data in blockmean and blockmode. This PR fixes the problem. blockmode was wrapped in GenericMappingTools#1456 (after v0.4.1), blockmean was wrapped in GenericMappingTools#1092 but the wrong alias was added in GenericMappingTools#1500 (after v0.4.1). Thus, the change won't go into the deprecation cycle.
Description of proposed changes
Wrap blockmode
Addresses #1091
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Slash Commands
You can write slash commands (
/command
) in the first line of a comment to performspecific operations. Supported slash commands are:
/format
: automatically format and lint the code/test-gmt-dev
: run full tests on the latest GMT development version