ENH: add disjoint_subset_union and disjoint_subset_union_all #1982
Conversation
shapely/set_operations.py
Outdated
| if isinstance(a, Geometry | None) and isinstance(b, Geometry | None): | ||
| pass | ||
| elif isinstance(a, Geometry | None): | ||
| a = np.full_like(b, a) | ||
| elif isinstance(b, Geometry | None): | ||
| b = np.full_like(a, b) | ||
| elif len(a) != len(b): | ||
| raise ValueError("Arrays a and b must have the same length") |
There was a problem hiding this comment.
Since disjoint_subset_union operates on a GeometryCollection, we need to roll-out custom broadcasting here.
Pull Request Test Coverage Report for Build 7878233005Details
💛 - Coveralls |
| if (isinstance(a, Geometry) or a is None) and ( | ||
| isinstance(b, Geometry) or b is None | ||
| ): | ||
| pass | ||
| elif isinstance(a, Geometry) or a is None: | ||
| a = np.full_like(b, a) | ||
| elif isinstance(b, Geometry) or b is None: | ||
| b = np.full_like(a, b) | ||
| elif len(a) != len(b): | ||
| raise ValueError("Arrays a and b must have the same length") |
There was a problem hiding this comment.
The logic looks good here (to mimic broadcasting), just wondering if this is already covered by the standard tests we have? (did you have to add this to get the tests passing?)
There was a problem hiding this comment.
Ah, and it might be that this can be simplified as a, b = np.broadcast_arrays(a, b)
There was a problem hiding this comment.
I tried broadcast_array before writing this and it does not work as we need it. Some of the tests fail with TypeError: One of the arguments is of incorrect type. Please provide only Geometry objects.
And yes, I had to include this to make the tests green, so it is properly covering by existing tests.
Exposing
GEOSDisjointSubsetUnionasdisjoint_subset_unionanddisjoint_subset_union_all, matching the API of union and coverage_union (assuming coverage_unoin will be fixed as discussed in #1963).Tests are currently only reusing the tooling for other set operations. Not sure if there's a need to go beyond that.
xref #1510