-
Notifications
You must be signed in to change notification settings - Fork 41
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 basic geometry conversion #260
Conversation
Codecov Report
@@ Coverage Diff @@
## main #260 +/- ##
==========================================
+ Coverage 96.54% 96.72% +0.18%
==========================================
Files 16 18 +2
Lines 1939 2110 +171
==========================================
+ Hits 1872 2041 +169
- Misses 67 69 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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 took a quick look; we'll need
- update api.rst
- tests
- make sure ci passes no-optional-deps
I think we can push better docs until we redo the documentation (I have a WIP branch)
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
…/cf-xarray into basic-geometry-conversion
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.
Hey I can't really review this so you should merge when you think it's ready. It would be good to fix some of those codecov comments.
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
Good! I'll continue some more testing in the next week. I see that we have no release coming soon, so I guess there is no rush! |
This adds a
geometry
submodule with 2 main functions :cf_to_geometry
andgeometry_to_cf
. Instead of implementing something geopandas-dependent, I realized I could just assume everything was xarray. That's just an addedgdf.to_xarray()
in my use case.geometry_to_cf
takes a 1D DataArray of shapely objects and translates it to a dataset that follows the CF conventions (specifically this part). In fact, it simply wrapspoints_to_cf
."Features" (as in individual geometries) have nodes and coordinates. Nodes names are easy : 'x' and 'y'. On the other hand, coordinates would normally have names depending on the grid mapping. Also, many attributes would depend on the grid mapping (units and standard_name for example, in nodes as well as coordinates). That's something we lost when not using
geopandas
, so I added agrid_mapping
keyword, but only the basiclongitude_latitude
is implemented.cf_to_geometry
implements the inverse conversion.shapely
is explicitly needed for this, but I didn't want to add a dependency, so import is done within the function. A DataArray of shapely objects is returned.For a complete
cfxr.geometry
, I think we would need:geometry_to_cf
.lines_to_cf
,polygons_to_cf
and vice-versa.Bonus
I don't know where to put
reshape_unique_geometries
... The idea is that our main use case of all this is to translate point timeseries to xarray objects. The input GeoDataframe is 1D, so if, for example, we get multiple timesteps for the same geometry, that geometry is repeated. Thereshape_unique_geometries
takes care of putting all unique geometries in a new axis. I guess it could be useful for people, but not sure if cf_xarray is the best place. And neither is xarray directly...How to use
And the way back:
I'll add tests next week.