-
Notifications
You must be signed in to change notification settings - Fork 10
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
jgrss/features delayed #250
Conversation
) as src: | ||
src_clip = gw.clip(src, l8_224078_20200518_polygons) | ||
self.assertEqual(src_clip.crs, src.crs) | ||
|
||
def test_clip_data(self): |
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.
New tests
@@ -903,8 +950,48 @@ def apply(self, filename, user_func, n_jobs=1, **kwargs): | |||
|
|||
cluster.stop() | |||
|
|||
def clip_by_polygon( |
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.
New method name
src/geowombat/core/geoxarray.py
Outdated
mask_data=mask_data, | ||
expand_by=expand_by, | ||
) | ||
|
||
def clip(self, df, query=None, mask_data=False): |
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.
Deprecated method
@@ -851,7 +903,14 @@ def extract( | |||
|
|||
return df | |||
|
|||
def clip(self, data, df, query=None, mask_data=False, expand_by=0): | |||
def clip_by_polygon( |
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.
New method name
|
||
return data | ||
|
||
def clip( |
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.
Deprecated method
not sure if this is where you want it
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. Check comment on clip
I also added clip to the read the docs (and fixed some typos in ML and added pygis into the external examples)
What's going on with CI build errors?
@@ -59,6 +59,7 @@ docs = numpydoc | |||
tests = testfixtures |
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.
In the read the docs we have an "all" option is it worth putting that back in?
@@ -903,8 +950,48 @@ def apply(self, filename, user_func, n_jobs=1, **kwargs): | |||
|
|||
cluster.stop() | |||
|
|||
def clip_by_polygon( | |||
self, | |||
df: T.Union[str, _Path, gpd.GeoDataFrame], |
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.
Just for my own edification, what is T.Union
for here? are you setting the required data types as string path or geodataframe? Then it handles error is there's datatype issues?
src/geowombat/core/geoxarray.py
Outdated
|
||
return clip(self._obj, df, query=query, mask_data=mask_data) | ||
return clip_by_polygon(self._obj, df, query=query, mask_data=mask_data) |
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 like despite warning its still running, so why not include expand_by=expand_by,
?
doc/source/tutorial-edit.rst
Outdated
@@ -164,3 +164,27 @@ The GeoWombat :func:`replace` function mimics :func:`pandas.DataFrame.replace`. | |||
.. note:: | |||
|
|||
The :func:`replace` function is typically used with thematic data. | |||
|
|||
Clipping to bounds |
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.
Not sure if this is where you want this.
Thanks! I'm not sure but I'll look into it as soon as I can. |
* fix: jgrss/features delayed (#250) * add flake8 * add black * add pre-commit * flake * flake * format * format * format * test * format * format * format * updating minor errors * adding pygis to examples * add clipping by polygons not sure if this is where you want it * move clip example * add missing arg * format docs * formatting * format --------- Co-authored-by: mmann1123 <mmann1123@gmail.com> * fix: CI tests failing with co-registration (#254) * add raise * wrong name * update CI workflow * pin max pyproj version * format * revert test * docs: Add CONTRIBUTING (#253) * block isort * block isort * add vscode ext notes * updating instructions (not complete) * in progress * add video test embed * change video image * play icon * video play button * play button * cleanup * cleanup --------- Co-authored-by: Michael Mann <mmann1123@gmail.com> * docs: Doc theme (#256) * remove jax * change html theme * remove clutter * update docs theme * bump python * remove sys * pin pydata-sphinx-theme * pin sphinx libs * make dict * freeze code block * format * format * format * format * format * format * format * format * format * format * format * format * format * format * format * format * format * format * format * format * format * format * format * format * format * format * format * pass tests * fix: Test coverage (#255) * format * format * format --------- Co-authored-by: mmann1123 <mmann1123@gmail.com>
What is this PR changing?
This PR address #249 and fixes the
clip()
method, deprecatesclip()
in favor ofclip_by_polygon()
, adds a test for clipping, and ensures clip masking is delayed.