Jg 541 subset spatial update #563

JanisGailis · 2018-03-15T18:51:35Z

Quite important changes to subset_spatial_impl. Also some rather minor changes in ds implementations to make sure we use the same implementation methods for the same tasks.

Closes #541
We can now select as well as plot 'long' polygons. Using explicit coordinates instead of polygons should now 'always' work as expected.

Closes #508
Performance of subset_spatial() is dramatically improved. Non-masked polygons will always use xarray selection machinery, Masked polygons first subset the dataset to polygon extents and then performs fast masking.

Closes #560
subset_spatial() now selects all pixels crossed by a given polygon. Both when xarray select is used, as well as with masking enabled.

Closes #507
subset_spatial() Now works with polygons smaller than pixels. Selecting either a single pixel, or in case it includes a pixel vertice or edge - crossed pixels. It also works when the provided dataset contains only a single pixel. If subset_spatial() is invoked with a polygon that does not cross the spatial extents of the given dataset, it will raise an appropriate error message.

Closes #559
plot_map() now by default produces a 'pixel' instead of a 'contour' plot. It is still possible to plot contours, by setting the appropriate parameter to 'True'.

Maybe something else I have forgotten.

Original extents information is preserved in subset_spatial implementation if a tuple or a string giving four values is used for PolygonLike. Plotting extents are fixed accordingly. Closes #541

Switch the dataset implementations to use the new spatial subsetting implementation letting the user select and retrieve 'long' rectangles.

Don't create a dis-joint longitude when making subsets over anti-meridian. Cate framework (making a workflow) relays on equi-distant longitude.

Adjust attributes in ds implementations using the common utility function to ensure correct extents being used when subsetting over the antimeridian. Fix subset tests

Let the user use complex polygons that cross anti-meridian if masking is False. Crop the dataset before masking.

A drastic improvement in performance from 4s -> 0.1s Fixes #508

Switch plotting to using meshgrid(plot pixels) instead of making a contour plot. Pad the subset bounding box to include crossed pixels

When an arbitrary polygon is provided as input to subset_spatial() and mask==True, the resulting dataset will feature all pixels that are crossed by the given polygon.

Add tests for new edge cases, clean-up implementation from debug messages

codecov-io · 2018-03-16T09:57:27Z

Codecov Report

❗ No coverage uploaded for pull request base (master@5074265). Click here to learn what that means.
The diff coverage is 80.86%.

@@            Coverage Diff            @@
##             master     #563   +/-   ##
=========================================
  Coverage          ?   76.65%           
=========================================
  Files             ?       78           
  Lines             ?    11278           
  Branches          ?        0           
=========================================
  Hits              ?     8645           
  Misses            ?     2633           
  Partials          ?        0

Impacted Files	Coverage Δ
cate/ops/animate.py	`24.06% <0%> (ø)`
cate/core/types.py	`94.26% <100%> (ø)`
cate/ds/esa_cci_odp.py	`50.56% <100%> (ø)`
cate/ds/local.py	`80.37% <100%> (ø)`
cate/ops/subset.py	`94.73% <100%> (ø)`
cate/ops/normalize.py	`100% <100%> (ø)`
cate/ops/plot_helpers.py	`85.49% <30%> (ø)`
cate/ops/plot.py	`29.58% <9.09%> (ø)`
cate/core/opimpl.py	`97.1% <96.46%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5074265...2aff50d. Read the comment docs.

forman · 2018-03-16T14:03:56Z

Thanks Janis. What a PR. Could you next time please limit PRs to one per issue, otherwise review takes too much time.

forman · 2018-03-16T14:09:45Z

cate/core/opimpl.py


    # Validate the bounding box
    if (not (-90 <= lat_min <= 90)) or \
            (not (-90 <= lat_max <= 90)) or \
            (not (-180 <= lon_min <= 180)) or \
            (not (-180 <= lon_max <= 180)):
-        raise ValueError('Provided polygon extent outside of geospatial'
+        raise ValueError('Provided polygon extends outside of geospatial'


Please use "extent", see http://www.dictionary.com/browse/extent

Ah... I don't agree. :)
The depends if you want a statement, or a description. But even if it's a statement, just 'extent' would be wrong-ish. So it's either:
'The provided polygon extent is outside of bounds'

Or with a verb (http://www.dictionary.com/browse/extend)
'The provided polygon extends outside of bounds'

JanisGailis · 2018-03-16T14:20:07Z

Sorry for the PR being so big feature wise. It's just that the issues were very interconnected.

Forgot to update CHANGES.md... Doing it now.

forman

Ok, looks good!

JanisGailis · 2018-03-16T15:00:17Z

@kbernat It'd be cool if you can take a look at the changes this brings to ds implementations. In a sentence - subset_spatial_impl now works on a PolygonLike and tries to be smart about it, so, there's no need to convert the region in ds classes, rather, the PolygonLike just gets passed on subset_spatial_impl instead. Also, use adjust_spatial_attrs instead of determining dataset extents manually, to keep the same concern in the same place.

If you're too busy today, I'll merge it anyway at the end of day and help you fix things in case this messes anything up (it shouldn't).

kbernat

good job!

Jānis Gailis added 25 commits March 8, 2018 14:52

Subset long_polygons

212e778

Original extents information is preserved in subset_spatial implementation if a tuple or a string giving four values is used for PolygonLike. Plotting extents are fixed accordingly. Closes #541

Fix animation

64b1308

Fix subset tests

178114f

ds uses new spatial_subset_impl

2486d9d

Switch the dataset implementations to use the new spatial subsetting implementation letting the user select and retrieve 'long' rectangles.

Don't create dis-joint longitude

541e1ee

Don't create a dis-joint longitude when making subsets over anti-meridian. Cate framework (making a workflow) relays on equi-distant longitude.

Adjust attributes, fix tests

ae84985

Adjust attributes in ds implementations using the common utility function to ensure correct extents being used when subsetting over the antimeridian. Fix subset tests

Notebook to test #508

add09d6

Subset spatial optimizations

96e2dfc

Let the user use complex polygons that cross anti-meridian if masking is False. Crop the dataset before masking.

Fix tests

1021215

Use matplotlib bulk ray tracing algorithm

ae384ad

A drastic improvement in performance from 4s -> 0.1s Fixes #508

Merge branch 'master' into jg-541-subset-spatial-update

0d0e132

Fix merging issues

548470a

More robust bounds handling

3766b91

Add monitor to subset_spatial

1654754

Plot meshgrid, include crossed pixels

740944a

Switch plotting to using meshgrid(plot pixels) instead of making a contour plot. Pad the subset bounding box to include crossed pixels

Update subset tests

9f01369

Select all crossed pixels when masking a polygon

ec8f6ff

When an arbitrary polygon is provided as input to subset_spatial() and mask==True, the resulting dataset will feature all pixels that are crossed by the given polygon.

Add parameter for pixel or contour plot

05c3638

Default pixel mesh for animations

9f9efdf

Handle selecting single pixels and 1D lines

922786b

Fix more edge cases

ba13167

Fix anomaly test

95a34a3

Tests, clean-up

5ade4c2

Add tests for new edge cases, clean-up implementation from debug messages

Notebok showcasing subset_spatial

1863c86

Fix tests, add allow_point parameter

0b5d621

JanisGailis self-assigned this Mar 15, 2018

Jānis Gailis added 3 commits March 15, 2018 19:59

Merge branch 'master' into jg-541-subset-spatial-update

c20d755

Fix ds implementations

945a4d2

Fix messed up automatic merge

17e74c6

JanisGailis requested a review from kbernat March 16, 2018 09:52

JanisGailis requested review from forman and mzuehlke March 16, 2018 09:53

forman reviewed Mar 16, 2018

View reviewed changes

forman approved these changes Mar 16, 2018

View reviewed changes

Jānis Gailis added 2 commits March 16, 2018 15:45

Flake8 fixes

b92fc86

Update CHANGES.md

b7388c0

Merge branch 'master' into jg-541-subset-spatial-update

2aff50d

kbernat approved these changes Mar 16, 2018

View reviewed changes

JanisGailis merged commit 5fdf25e into master Mar 17, 2018

JanisGailis deleted the jg-541-subset-spatial-update branch March 17, 2018 01:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jg 541 subset spatial update #563

Jg 541 subset spatial update #563

JanisGailis commented Mar 15, 2018 •

edited

Loading

codecov-io commented Mar 16, 2018 •

edited

Loading

forman commented Mar 16, 2018

forman Mar 16, 2018

JanisGailis Mar 16, 2018 •

edited

Loading

JanisGailis commented Mar 16, 2018 •

edited

Loading

forman left a comment

JanisGailis commented Mar 16, 2018

kbernat left a comment

Jg 541 subset spatial update #563

Jg 541 subset spatial update #563

Conversation

JanisGailis commented Mar 15, 2018 • edited Loading

codecov-io commented Mar 16, 2018 • edited Loading

Codecov Report

forman commented Mar 16, 2018

forman Mar 16, 2018

Choose a reason for hiding this comment

JanisGailis Mar 16, 2018 • edited Loading

Choose a reason for hiding this comment

JanisGailis commented Mar 16, 2018 • edited Loading

forman left a comment

Choose a reason for hiding this comment

JanisGailis commented Mar 16, 2018

kbernat left a comment

Choose a reason for hiding this comment

JanisGailis commented Mar 15, 2018 •

edited

Loading

codecov-io commented Mar 16, 2018 •

edited

Loading

JanisGailis Mar 16, 2018 •

edited

Loading

JanisGailis commented Mar 16, 2018 •

edited

Loading