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

Add grid_bounds_to_polygons #478

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Add grid_bounds_to_polygons #478

wants to merge 15 commits into from

Conversation

dcherian
Copy link
Contributor

@dcherian dcherian commented Oct 26, 2023

  • Add to api.rst
  • Add test

This converts "bounds" in the 2-element form to an array of shapely polygons, one per grid cell. A step towards nicely packaging conservative regridding for the ecosystem. It could easily be generalized to the 4-element "vertex" form

Modified from
https://notebooksharing.space/view/c6c1f3a7d0c260724115eaa2bf78f3738b275f7f633c1558639e7bbd75b31456#displayOptions=

cc @martinfleis @benbovy @aulemahal for awareness. I'm not sure if this is at all useful for xvec but on the off-chance that it is..

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Attention: Patch coverage is 10.34483% with 26 lines in your changes missing coverage. Please review.

Project coverage is 83.06%. Comparing base (a9cebee) to head (d33610e).
Report is 14 commits behind head on main.

Current head d33610e differs from pull request most recent head 0291802

Please upload reports for the commit 0291802 to get more accurate results.

Files Patch % Lines
cf_xarray/geometry.py 7.14% 26 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #478      +/-   ##
==========================================
- Coverage   85.78%   83.06%   -2.73%     
==========================================
  Files          13       14       +1     
  Lines        2364     2799     +435     
  Branches      183      208      +25     
==========================================
+ Hits         2028     2325     +297     
- Misses        303      423     +120     
- Partials       33       51      +18     
Flag Coverage Δ
mypy 43.75% <10.34%> (+5.22%) ⬆️
unittests 92.89% <7.14%> (-1.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

cf_xarray/geometry.py Outdated Show resolved Hide resolved
cf_xarray/geometry.py Outdated Show resolved Hide resolved
@martinfleis
Copy link
Member

In Xvec we could flatten the array and use it as an index instead of latitude and longitude coordinates if there's a use case for that.

Copy link
Contributor

@mathause mathause left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks very promising also for regionmask as we discussed in regionmask/regionmask#499. Is it a general assumption in cf_xarray that your grid is lat/ lon in degree?

cf_xarray/geometry.py Outdated Show resolved Hide resolved
cf_xarray/geometry.py Outdated Show resolved Hide resolved
cf_xarray/geometry.py Outdated Show resolved Hide resolved
cf_xarray/geometry.py Outdated Show resolved Hide resolved
Comment on lines +494 to +496
# geopandas needs this
mask = lonbnd[..., 0] >= 180
lonbnd[mask, :] = lonbnd[mask, :] - 360
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the reason for this is that most geographic data is given in WGS84 which is -180...180 (I may be wrong here). So if you want to compare it to regional polygons it is probably a good idea to wrap the data. Still, it may be confusing to users.

Maybe this could be optional. However, I am not sure what a good name is (In regionmask I call it wrap_lon with the options 180 (= what you do here), 360 and False. That works but there may be better names).

dcherian and others added 4 commits June 17, 2024 16:02
Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>
Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>
cf_xarray/geometry.py Outdated Show resolved Hide resolved
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.

None yet

4 participants