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

Questionable behaviour of symmetric_difference_all #2027

Open
martinfleis opened this issue Mar 25, 2024 · 4 comments
Open

Questionable behaviour of symmetric_difference_all #2027

martinfleis opened this issue Mar 25, 2024 · 4 comments
Milestone

Comments

@martinfleis
Copy link
Contributor

My understanding of symmetric_difference_all is that it returns all parts of geometries that are unique to a single one, i.e. those parts that do not intersect any other. But that doesn't seem to be the case as shown in the example below.

import numpy as np
import shapely

geoms = np.array(
    [box(0, 0, 2, 2), box(1, 1, 3, 3), box(0, 0, 1.5, 1.5)]
)
shapely.symmetric_difference_all(geoms)

Visually:

You can see that the small square in the middle is shared by all three polygons.
Screenshot 2024-03-25 at 14 30 21
Yet, it is present in the result.
Screenshot 2024-03-25 at 14 31 21

I would expect the result to not contain that small square. Is this my misunderstanding or a bug?

@theroggy
Copy link
Contributor

theroggy commented Mar 25, 2024

I also think this is not the expected result, so I'd think it is a bug.

I had a (very) quick look on how it is implemented, and the same Y_Y_reduce_func is used as is used as for intersection_all. I didn't look into details how the c function works, but I suppose this implements a loop through all inputs where each input geometry is, for symmetric difference_all, symmetric differenced from the result of the previous two... which would indeed lead to this (incorrect) result.

@jorisvandenbossche jorisvandenbossche added this to the 2.1 milestone Apr 29, 2024
@jorisvandenbossche
Copy link
Member

Indeed, you essentially can't do this iteratively like we do now, in each step you need to consider all other geometries.

The very dumb way I can currently think of is that for each geometry, you calculate the sym diff with the union (or just collection) of all other geometries. That sounds quite expensive, but not sure if there is a way to optimize this? (one could use a spatial index to do the sym diff with only the relevant (intersecting) other geometries)

@martinfleis did you have an actual use case for this, or just looking at it to consider adding the equivalent to geopandas?
I am wondering if we shouldn't rather deprecate/remove this from shapely given it is not actually correct.

@jorisvandenbossche
Copy link
Member

cc @caspervdw

@martinfleis
Copy link
Contributor Author

@martinfleis did you have an actual use case for this, or just looking at it to consider adding the equivalent to geopandas?
I am wondering if we shouldn't rather deprecate/remove this from shapely given it is not actually correct.

No use case. I hit this while trying to expose it on geopandas side. Deprecation is fine with me.

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

No branches or pull requests

3 participants