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

variable geometry support, plotting, WKB IO #98

Merged
merged 51 commits into from
Feb 21, 2025

Conversation

martinfleis
Copy link
Member

This is a bit of a best but it got too interlinked that breaking it apart would be pain. It has came to live during a week-long workshop with @loreabad6 and I needed to use the time with her to get the functionality in.

So, this PR is a quite a milestone for Xvec as it:

  1. adds support for variable geometry (shapely geometry as data in the array, not only as coordinates). Think about moving geometries, time evolving (e.g. glaciers) and so on.
  2. add support to summarise variable geometry using GeometryIndex
  3. add support for plotting of all kinds of data cubes
  4. add encoding and decoding to WKB that supports Zarr serialisation of variable geometry
  5. expands zonal stats so it can be used with variable geometry on input

Tests are still mostly missing as is documentation, so WIP.

@benbovy to store the CRS of variable geometry, I opted for xproj as it does precisely what I needed. Is it a good idea? I made sure we don't depend on it yet.

@benbovy
Copy link
Member

benbovy commented Feb 21, 2025

@benbovy to store the CRS of variable geometry, I opted for xproj as it does precisely what I needed. Is it a good idea? I made sure we don't depend on it yet.

Yes I think it is a good idea :-) !

I haven't looked closely into this massive PR, though. From what I see you use the .proj.crs property to access the CRS, which assumes a unique CRS for the whole Dataset or DataArray.

BTW I plan to submit another Xvec PR in order to make xvec.GeometryIndex CRS-aware from xproj's point of view, which does not require depending on xproj either. One possible caveat is that if xproj finds several GeometryIndex coordinates with different CRSs in the DataArray / Dataset, .proj.crs will raise an error.

In CF conventions any data variable having (horizontal) spatial dimensions can be linked to a spatial reference (scalar) variable via its grid_mapping attribute, but xproj does not handle that yet (I opened benbovy/xproj#21).

@benbovy
Copy link
Member

benbovy commented Feb 21, 2025

This PR looks amazing!

Copy link

codecov bot commented Feb 21, 2025

Codecov Report

Attention: Patch coverage is 40.98361% with 144 lines in your changes missing coverage. Please review.

Project coverage is 80.91%. Comparing base (66b541b) to head (4df37db).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
xvec/plotting.py 13.67% 101 Missing ⚠️
xvec/zonal.py 6.25% 30 Missing ⚠️
xvec/accessor.py 86.31% 13 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main      #98       +/-   ##
===========================================
- Coverage   98.70%   80.91%   -17.80%     
===========================================
  Files           4        5        +1     
  Lines         540      791      +251     
===========================================
+ Hits          533      640      +107     
- Misses          7      151      +144     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@martinfleis martinfleis marked this pull request as ready for review February 21, 2025 15:12
@martinfleis martinfleis merged commit f2962d4 into xarray-contrib:main Feb 21, 2025
9 checks passed
@martinfleis martinfleis deleted the summary branch February 21, 2025 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants