grdfilter wrapper #612

carocamargo · 2020-09-17T19:41:52Z

Added grdfilter function to the gridops.py file, and modified the .init.py to call the new function as well.

Grdfilter is a wrapper of the grdfilter function of GMT.
Given an input grid (either a datarray or a netcdf file), it applies a filter (of chosen type and length), and returns the filtered field.
Grdfilter function was made by modifying the grdcut function.

welcome · 2020-09-17T19:41:54Z

💖 Thanks for opening this pull request! 💖

Please make sure you read our contributing guidelines and abide by our code of conduct.

A few things to keep in mind:

If you need help writing tests, take a look at the existing ones for inspiration. If you don't know where to start, let us know and we'll walk you through it.
All new features should be documented. It helps to write the docstrings for your functions/classes before writing the code. This will help you think about your code design and results in better code.
No matter what, we are really grateful that you put in the effort to do this! 🎉

carocamargo · 2020-09-17T19:51:17Z

I think the changes in the init.py were blocked. But it's just to add call gridfilter
from .gridops import grdcut, gridfilter

weiji14 · 2020-09-17T20:51:54Z

pygmt/__init__.py

@@ -19,7 +19,7 @@
 from .sampling import grdtrack
 from .mathops import makecpt
 from .modules import GMTDataArrayAccessor, config, info, grdinfo, which
-from .gridops import grdcut
+from .gridops import grdcut, gridfilter


Suggested change

from .gridops import grdcut, gridfilter

from .gridops import grdcut, grdfilter

Just a small typo, that's why it's not importing 😃

weiji14 · 2020-09-17T21:21:54Z

Thanks for opening this PR @carocamargo. Once you fix the typo, you should be able to start using pygmt.grdfilter directly after pip install https://github.com/carocamargo/pygmt/archive/master.zip (and others are welcome to try too).

Let me know how much you want to work on this PR, depending on your time/Python familiarity:

Level 1. Just happy that it works, and want to get on with other stuff.
Level 2. Happy to make changes to the docstring, and maybe a couple of minor changes here and there.
Level 3. Write up a few unit tests (see example of grdcut PR at #492), and possibly add a gallery example.

seisman

Another issue is that you're making changes to your "master" branch. It's OK but it will cause troubles after we merging this PR. It's always better to make changes in a new branch.

seisman · 2020-09-17T21:19:06Z

pygmt/gridops.py

@@ -20,13 +20,15 @@

 @fmt_docstring
 @use_alias(


You're changing the alias settings for grdcut. That's incorrect. You should revert the changes here and add decorators @fmt_docstring, @use_alias and @kwargs_to_strings just before the grdfilter function, i.e., each function has its own decorators.

Ok.. I thought they could be the same. Thanks!

seisman · 2020-09-17T21:19:48Z

pygmt/gridops.py

+    """
+
+        filter a grid file in the time domain using one of the selected convolution
+        or non-convolution isotropic or rectangular filters and compute distances
+        using Cartesian or Spherical geometries. The output grid file can optionally
+        be generated as a sub-region of the input (via -R) and/or with new increment
+        (via -I) or registration (via -T). In this way, one may have “extra space” in
+        the input data so that the edges will not be used and the output can be within
+        one half-width of the input edges. If the filter is low-pass, then the output
+        may be less frequently sampled than the input.
+
+        Parameters
+        ----------
+        grid : str or xarray.DataArray
+        The file name of the input grid or the grid loaded as a DataArray.
+        outgrid : str or None
+        The name of the output netCDF file with extension .nc to store the grid
+        in.
+        {F} : str
+        Name of filter type you which to apply, followed by the width
+        b: Box Car; c: Cosine Arch; g: Gaussian; o: Operator; m: Median; p: Maximum Likelihood probability; h: histogram
+        {D}: str
+        Distance flag, that tells how grid (x,y) rrlated to the filter width as follows:
+        flag = p: grid (px,py) with width an odd number of pixels; Cartesian distances.
+
+        flag = 0: grid (x,y) same units as width, Cartesian distances.
+
+        flag = 1: grid (x,y) in degrees, width in kilometers, Cartesian distances.
+
+        flag = 2: grid (x,y) in degrees, width in km, dx scaled by cos(middle y), Cartesian distances.
+
+        The above options are fastest because they allow weight matrix to be computed only once. The next three options are slower because they recompute weights for each latitude.
+
+        flag = 3: grid (x,y) in degrees, width in km, dx scaled by cosine(y), Cartesian distance calculation.
+
+        flag = 4: grid (x,y) in degrees, width in km, Spherical distance calculation.
+
+        flag = 5: grid (x,y) in Mercator -Jm1 img units, width in km, Spherical distance calculation.
+
+        Returns
+        -------
+        ret: xarray.DataArray or None
+        Return type depends on whether the *outgrid* parameter is set:
+        - xarray.DataArray if *outgrid* is not set
+        - None if *outgrid* is set (grid output will be stored in *outgrid*)
+        """


The indentation of the docstrings looks wrong.

carocamargo · 2020-09-18T07:48:39Z

Another issue is that you're making changes to your "master" branch. It's OK but it will cause troubles after we merging this PR. It's always better to make changes in a new branch.

I'm new to all github things.. This is the only way that I know how to commit the changes...

…with each owns decorators

carocamargo · 2020-09-18T08:20:37Z

I corrected the things as suggested by @seisman and @weiji14, and made a new PR, now from a new branch (I believe... I'm still learning to use github correctly).

carocamargo · 2020-09-18T08:23:27Z

Thanks for opening this PR @carocamargo. Once you fix the typo, you should be able to start using pygmt.grdfilter directly after pip install https://github.com/carocamargo/pygmt/archive/master.zip (and others are welcome to try too).

Let me know how much you want to work on this PR, depending on your time/Python familiarity:

Level 1. Just happy that it works, and want to get on with other stuff.
Level 2. Happy to make changes to the docstring, and maybe a couple of minor changes here and there.
Level 3. Write up a few unit tests (see example of grdcut PR at #492), and possibly add a gallery example.

I wouldn't mind working on this function, even up to Level 3... but as you can see my github knowledge is quite limited

weiji14 · 2020-09-18T10:38:40Z

I wouldn't mind working on this function, even up to Level 3... but as you can see my github knowledge is quite limited

Excellent, we'll try to guide you along then (let us know if you have any questions), there's always a first time for everyone 😄

I'll close this PR since you've got a parallel one open at #616.

carocamargo added 2 commits September 17, 2020 21:36

added gridfilter function

884bde7

added to call gridfilter from grdops.py

c2a7d3e

vercel bot temporarily deployed to Preview September 17, 2020 19:41 Inactive

weiji14 reviewed Sep 17, 2020

View reviewed changes

weiji14 added feature request New feature wanted feature Brand new feature and removed feature request New feature wanted labels Sep 17, 2020

seisman reviewed Sep 17, 2020

View reviewed changes

added grdfilter caller

2cb715d

vercel bot temporarily deployed to Preview September 18, 2020 07:44 Inactive

corrected indentation in grdfilter docstring, and made each function …

6139c17

…with each owns decorators

vercel bot temporarily deployed to Preview September 18, 2020 07:53 Inactive

weiji14 closed this Sep 18, 2020

carocamargo mentioned this pull request Sep 18, 2020

Wrap grdfilter #616

Merged

5 tasks

weiji14 mentioned this pull request Oct 21, 2020

-JEPSG:4326 not writing CRS to netcdf in surface #655

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

grdfilter wrapper #612

grdfilter wrapper #612

carocamargo commented Sep 17, 2020

welcome bot commented Sep 17, 2020

carocamargo commented Sep 17, 2020

weiji14 Sep 17, 2020

weiji14 commented Sep 17, 2020

seisman left a comment

seisman Sep 17, 2020

carocamargo Sep 18, 2020

seisman Sep 17, 2020

carocamargo commented Sep 18, 2020

carocamargo commented Sep 18, 2020

carocamargo commented Sep 18, 2020

weiji14 commented Sep 18, 2020

	from .gridops import grdcut, gridfilter
	from .gridops import grdcut, grdfilter

grdfilter wrapper #612

grdfilter wrapper #612

Conversation

carocamargo commented Sep 17, 2020

welcome bot commented Sep 17, 2020

carocamargo commented Sep 17, 2020

weiji14 Sep 17, 2020

Choose a reason for hiding this comment

weiji14 commented Sep 17, 2020

seisman left a comment

Choose a reason for hiding this comment

seisman Sep 17, 2020

Choose a reason for hiding this comment

carocamargo Sep 18, 2020

Choose a reason for hiding this comment

seisman Sep 17, 2020

Choose a reason for hiding this comment

carocamargo commented Sep 18, 2020

carocamargo commented Sep 18, 2020

carocamargo commented Sep 18, 2020

weiji14 commented Sep 18, 2020