Skip to content

Allow n-dimensional arrays into polygonize in DD #253

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: dd
Choose a base branch
from
Open

Allow n-dimensional arrays into polygonize in DD #253

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions ext/GeometryOpsDimensionalDataExt/GeometryOpsDimensionalDataExt.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
import GeometryOps as GO
import GeoInterface as GI

function GO.polygonize(A::DD.AbstractDimArray; dims=(DD.X(), DD.Y()), crs=GI.crs(A), kw...)
function GO.polygonize(A::DD.AbstractDimArray{T, N}; dims=(DD.XDim, DD.YDim), crs=GI.crs(A), kw...) where {T, N}

Check warning on line 7 in ext/GeometryOpsDimensionalDataExt/GeometryOpsDimensionalDataExt.jl

View check run for this annotation

Codecov / codecov/patch

ext/GeometryOpsDimensionalDataExt/GeometryOpsDimensionalDataExt.jl#L7 

Added line #L7 was not covered by tests
# Extract the lookups of the dimensions we care about
lookups = DD.lookup(A, dims)
# Convert them to interval-bound vectors
bounds_vecs = if DD.isintervals(lookups)
map(DD.intervalbounds, lookups)
else
Expand All @@ -14,7 +16,24 @@
DD.intervalbounds(DD.set(l, DD.Intervals()))
end
end
GO.polygonize(bounds_vecs..., DD.AbstractDimArray; crs, kw...)

# This tree switches on the array type.
# If ndims < 2, then we can't polygonize anyway, so we error.
# If ndims > 2, then we return a DimArray across the remaining dimensions
# not provided in `dims`. Each slice in `dims` (usually X and Y)
if N < 2
error("Array had $N dimensions, but it must have at least two!")
elseif N == 2

Check warning on line 26 in ext/GeometryOpsDimensionalDataExt/GeometryOpsDimensionalDataExt.jl

View check run for this annotation

Codecov / codecov/patch

ext/GeometryOpsDimensionalDataExt/GeometryOpsDimensionalDataExt.jl#L24-L26 

Added lines #L24 - L26 were not covered by tests
# If ndims == 2, then we polygonize directly, and return the output as requested.
Ap = PermutedDimsArray(A, dims)
GO.polygonize(bounds_vecs..., Ap; crs, kw...)

Check warning on line 29 in ext/GeometryOpsDimensionalDataExt/GeometryOpsDimensionalDataExt.jl

View check run for this annotation

Codecov / codecov/patch

ext/GeometryOpsDimensionalDataExt/GeometryOpsDimensionalDataExt.jl#L28-L29 

Added lines #L28 - L29 were not covered by tests
else
map(eachslice(A; dims = DD.otherdims(A, dims))) do a
ap = PermutedDimsArray(a, dims)
GO.polygonize(bounds_vecs..., a; crs, kw...)

Check warning on line 33 in ext/GeometryOpsDimensionalDataExt/GeometryOpsDimensionalDataExt.jl

View check run for this annotation

Codecov / codecov/patch

ext/GeometryOpsDimensionalDataExt/GeometryOpsDimensionalDataExt.jl#L31-L33 

Added lines #L31 - L33 were not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just pass through the dims kw instead of permuting?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The x and y dims need to be in the same order as the lookups though, so I'm not sure how we'd do this...

Copy link
Member

@rafaqz rafaqz Jan 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't it already fix that with dims ordering? It probably should...

Maybe it's counter intuitive but reordering dims in the inner loop is essentially free, so usually faster than array wrappers that have a runtime Indirection

(Ohh right it's not a DD alg internally so it has to be slow... unfortunate why would I write something like that 😭)

end
end

end


Expand Down
Loading