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 blockmean #1092

Merged
merged 15 commits into from
Mar 23, 2021
Merged

Wrap blockmean #1092

merged 15 commits into from
Mar 23, 2021

Conversation

maxrjones
Copy link
Member

Description of proposed changes

This PR wraps the blockmean module. Some changes involved in this PR:

  • Most of the functionality from the blockmedian method is moved to _blockm
  • blockmedian and blockmean call _blockm with the block_method parameter
  • pygmt/src/blockmedian.py is renamed to pygmt/src/blockm.py for simplicity given the similarities between these three methods. I think this is simpler (more pythonic) than having each in separate modules and importing _blockm from a fourth module, but feedback is welcome if others feel that Should data processing functions have their own modules? #807 should be more strictly adhered to.
  • New tests for blockmean are added

Addresses #1091

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

@seisman seisman added this to the 0.4.0 milestone Mar 21, 2021
@seisman seisman added the feature Brand new feature label Mar 21, 2021
pygmt/src/blockm.py Outdated Show resolved Hide resolved
pygmt/src/blockm.py Outdated Show resolved Hide resolved
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 one very minor point on moving _blockm to the top of the file, otherwise well done!

pygmt/src/blockm.py Outdated Show resolved Hide resolved
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.

Thanks Meghan! Really appreciate having another data processing module added to PyGMT's toolkit 😄

@weiji14 weiji14 added the final review call This PR requires final review and approval from a second reviewer label Mar 22, 2021
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.

Looks good to me.

@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Mar 23, 2021
@seisman seisman merged commit 28d37ec into master Mar 23, 2021
@seisman seisman deleted the blockm branch March 23, 2021 19:36
seisman added a commit that referenced this pull request Oct 3, 2021
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.
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
* Rename blockmedian.py to blockm.py
* Refactor blockmedian to support mean and mode
* Wrap method blockmean
* Add tests for blockmean method

Co-authored-by: Dongdong Tian <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Brand new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants