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

Accessor #10

Closed
martinfleis opened this issue Nov 23, 2022 · 5 comments · Fixed by #11
Closed

Accessor #10

martinfleis opened this issue Nov 23, 2022 · 5 comments · Fixed by #11

Comments

@martinfleis
Copy link
Member

In some occasions (#4 #5) we mentioned that the accessor would be a nice addition here and could eventually expose a lot of what shapely/geopandas now offer.

What would be the ideal name, that is not ambiguous and minimises a risk of a conflict with another accessor by another package?

We mentioned .vec but that may not be the most intuitive. .geo is taken by geoxarray and does apply to both raster and vector anyway. What about using .geom indicating an operation on geometry? That would also be consistent with GeometryIndex.

@benbovy
Copy link
Member

benbovy commented Nov 23, 2022

Ideally it should also minimize the risk of a conflict with coordinate and variable names.

There's no real consensus regarding how to name xarray extension packages and accessors, it varies from one package to another.

If the package aims to provide a unique accessor, I'd tend to choose the same name for both the package and the accessor as it is easier to remember. I also like this pattern: xyz-xarray (with or without -) for the package name and xyz for the accessor name (like cf-xarray, pint-xarray and rioxarray). I don't have strong opinion, though.

@martinfleis
Copy link
Member Author

Ideally it should also minimize the risk of a conflict with coordinate and variable names.

True. That eliminates .geom as that will be one of the first choices for many when using shapely geoms as coordinates.

We can also consider following xoak's model and use .xvec as an accessor name. That is both unambiguous and quite unique as a string to minimise the potential conflict.

@benbovy
Copy link
Member

benbovy commented Nov 24, 2022

use .xvec as an accessor name

+1

@benbovy
Copy link
Member

benbovy commented Dec 2, 2022

It may be too premature, but I'm also wondering how would co-exist geometry and geography coordinates in a Dataset / DataArray?

Should we reuse the same xvec accessor? If yes, maybe have two geom_coords and geog_coords properties returning geometry / geography coordinates, respectively?

With the suggested API in https://github.com/martinfleis/xvec/pull/11#issuecomment-1334964188, any method called with coords=None may raise an error if both geometry and geography coordinate are found...

@martinfleis
Copy link
Member Author

I would try to match the behaviour of geopandas in relation to geometry/geography and that is not defined yet.

We can potentially raise for unsupported ops when geography is passed, I assume that geopandas will do that as well.

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 a pull request may close this issue.

2 participants