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

Dimension property: returned value for geography collections #57

Open
benbovy opened this issue Oct 21, 2024 · 3 comments · May be fixed by #87
Open

Dimension property: returned value for geography collections #57

benbovy opened this issue Oct 21, 2024 · 3 comments · May be fixed by #87
Labels
question Further information is requested upstream related to upstream dependencies (s2geography or s2geometry)

Comments

@benbovy
Copy link
Owner

benbovy commented Oct 21, 2024

In the case where a collection contains features of different dimensions, the default value returned via s2geography (-1) is not consistent with the value returned by shapely (max dimension value found).

Which one do we want?

Originally posted by @jorisvandenbossche in #51 (comment)

@jorisvandenbossche
Copy link
Collaborator

I am not entirely sure why s2geography returns -1 (cc @paleolimbot?), because AFAIK R's s2 returns the max dimension of the parts (except for an empty collection): https://r-spatial.github.io/s2/reference/s2_is_collection.html (see the examples).
And I suppose this is then also consistent with bigquery (I assume, based on the description, but didn't test that)

@jorisvandenbossche
Copy link
Collaborator

It seems that in s2geography, there is a difference between the Geography::dimension() attribute and the s2_dimension() function.

The attribute is used in code where you want to know if you have points or polylines or polygons (and if not one of those three, it returns -1), and then the logic to calculate the max dimension in case of collections is handled inside the s2_dimension function (https://github.com/paleolimbot/s2geography/blob/1c67ddc62c30977fabca291fb68fd8cb62f3beaa/src/s2geography/accessors.cc#L55-L69)

@benbovy benbovy added upstream related to upstream dependencies (s2geography or s2geometry) question Further information is requested labels Nov 14, 2024
@benbovy
Copy link
Owner Author

benbovy commented Dec 10, 2024

Yes I think we rather want the result from the s2geography::s2_dimension() function here, which returns values that are consistent with shapely.

IIUC bigquery's ST_DIMENSION always returns -1 for empty geography objects? Whereas shapely.get_dimensions() and s2geography::s2_dimension() both return 0, 1, 2 for empty point, linestring and polygon geography objects, respectively.

@benbovy benbovy linked a pull request Dec 10, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested upstream related to upstream dependencies (s2geography or s2geometry)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants