-
Notifications
You must be signed in to change notification settings - Fork 4
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
[WIP] Use GeometryOpsCore for real #223
base: main
Are you sure you want to change the base?
Changes from all commits
d9ff60b
f50c82a
324fccb
36bb92c
e40db7d
2009463
611a726
86f3b87
73f486b
1f5613a
b07808f
7ea0676
50cdbef
91ee1ca
739c55b
a41c257
53936f1
60593d6
3841ec8
6177640
b281ee6
a38f74a
2c79028
2f29e24
39b51f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
name = "GeometryOpsCore" | ||
uuid = "05efe853-fabf-41c8-927e-7063c8b9f013" | ||
authors = ["Anshul Singhvi <[email protected]>", "Rafael Schouten <[email protected]>", "Skylar Gering <[email protected]>", "and contributors"] | ||
version = "0.1.1" | ||
version = "0.1.2" | ||
|
||
[deps] | ||
DataAPI = "9a962f9c-6df0-11e9-0e5d-c546b8b5ee8a" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ module GeometryOpsLibGEOSExt | |
import GeometryOps as GO, LibGEOS as LG | ||
import GeoInterface as GI | ||
|
||
import GeometryOps: GEOS, enforce | ||
import GeometryOps: GEOS, enforce, _True, _False, _booltype | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feels like we should remove the underscores if we are importing these? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. We can probably make these uppercase, but only exported from Core (not GeometryOps proper)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep perfect There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just realised we can use all of these in Rasters.jl too as its all the same there already. So no underscores is good. Really keen to unify GeometryOps/Rasters around this core for all geometry related things now. |
||
|
||
using GeometryOps | ||
# The filter statement is required because in Julia, each module has its own versions of these | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,16 +2,16 @@ | |
|
||
module GeometryOps | ||
|
||
include("../GeometryOpsCore/src/GeometryOpsCore.jl") # TODO: replace this with `using GeometryOpsCore` | ||
import .GeometryOpsCore | ||
for name in setdiff(names(GeometryOpsCore, all = true), (:eval, :var"#eval", :include, :var"#include")) | ||
# Import all symbols from GeometryOpsCore | ||
@eval import .GeometryOpsCore: $name | ||
# Re-export all exported symbols | ||
if Base.isexported(GeometryOpsCore, name) | ||
@eval export $name | ||
end | ||
end | ||
import GeometryOpsCore | ||
import GeometryOpsCore: | ||
TraitTarget, | ||
Manifold, Planar, Spherical, Geodesic, | ||
BoolsAsTypes, _True, _False, _booltype, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again the underscores on imported objects... |
||
apply, applyreduce, | ||
flatten, reconstruct, rebuild, unwrap, _linearring, | ||
APPLY_KEYWORDS, THREADED_KEYWORD, CRS_KEYWORD, CALC_EXTENT_KEYWORD | ||
|
||
export TraitTarget, Manifold, Planar, Spherical, Geodesic, apply, applyreduce, flatten, reconstruct, rebuild, unwrap | ||
|
||
using GeoInterface | ||
using GeometryBasics | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -146,7 +146,7 @@ | |
# Add an error hint for GeodesicSegments if Proj is not loaded! | ||
function _geodesic_segments_error_hinter(io, exc, argtypes, kwargs) | ||
if isnothing(Base.get_extension(GeometryOps, :GeometryOpsProjExt)) && exc.f == GeodesicSegments | ||
print(io, "\n\nThe `GeodesicSegments` method requires the Proj.jl package to be explicitly loaded.\n") | ||
print(io, "\n\nThe `Geodesic` method requires the Proj.jl package to be explicitly loaded.\n") | ||
print(io, "You can do this by simply typing ") | ||
printstyled(io, "using Proj"; color = :cyan, bold = true) | ||
println(io, " in your REPL, \nor otherwise loading Proj.jl via using or import.") | ||
|
@@ -156,51 +156,64 @@ | |
# ## Implementation | ||
|
||
""" | ||
segmentize([method = LinearSegments()], geom; max_distance::Real, threaded) | ||
segmentize([method = Planar()], geom; max_distance::Real, threaded) | ||
|
||
Segmentize a geometry by adding extra vertices to the geometry so that no segment is longer than a given distance. | ||
This is useful for plotting geometries with a limited number of vertices, or for ensuring that a geometry is not too "coarse" for a given application. | ||
|
||
## Arguments | ||
- `method::SegmentizeMethod = LinearSegments()`: The method to use for segmentizing the geometry. At the moment, only [`LinearSegments`](@ref) and [`GeodesicSegments`](@ref) are available. | ||
- `geom`: The geometry to segmentize. Must be a `LineString`, `LinearRing`, or greater in complexity. | ||
- `max_distance::Real`: The maximum distance, **in the input space**, between vertices in the geometry. Only used if you don't explicitly pass a `method`. | ||
- `method::Manifold = Planar()`: The method to use for segmentizing the geometry. At the moment, only [`Planar`](@ref) (assumes a flat plane) and [`Geodesic`](@ref) (assumes geometry on the ellipsoidal Earth and uses Vincenty's formulae) are available. | ||
- `geom`: The geometry to segmentize. Must be a `LineString`, `LinearRing`, `Polygon`, `MultiPolygon`, or `GeometryCollection`, or some vector or table of those. | ||
- `max_distance::Real`: The maximum distance between vertices in the geometry. **Beware: for `Planar`, this is in the units of the geometry, but for `Geodesic` and `Spherical` it's in units of the radius of the sphere.** | ||
|
||
Returns a geometry of similar type to the input geometry, but resampled. | ||
""" | ||
function segmentize(geom; max_distance, threaded::Union{Bool, BoolsAsTypes} = _False()) | ||
return segmentize(LinearSegments(; max_distance), geom; threaded = _booltype(threaded)) | ||
return segmentize(Planar(), geom; max_distance, threaded = _booltype(threaded)) | ||
end | ||
|
||
# allow three-arg method as well, just in case | ||
segmentize(geom, max_distance::Real; threaded = _False()) = segmentize(Planar(), geom, max_distance; threaded) | ||
segmentize(method::Manifold, geom, max_distance::Real; threaded = _False()) = segmentize(Planar(), geom; max_distance, threaded) | ||
|
||
# generic implementation | ||
function segmentize(method::Manifold, geom; max_distance, threaded::Union{Bool, BoolsAsTypes} = _False()) | ||
@assert max_distance > 0 "`max_distance` should be positive and nonzero! Found $(method.max_distance)." | ||
_segmentize_function(geom) = _segmentize(method, geom, GI.trait(geom); max_distance) | ||
return apply(_segmentize_function, TraitTarget(GI.LinearRingTrait(), GI.LineStringTrait()), geom; threaded) | ||
Comment on lines
+180
to
+183
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like code duplication with the function below, are both needed? Are these asserts not checked twice when called from the other method? (Also throwing an error is better than asserts as it's guaranteed to actually run) |
||
end | ||
|
||
function segmentize(method::SegmentizeMethod, geom; threaded::Union{Bool, BoolsAsTypes} = _False()) | ||
@warn "`segmentize(method::$(typeof(method)), geom) is deprecated; use `segmentize($(method isa LinearSegments ? "Planar()" : "Geodesic()"), geom; max_distance, threaded) instead!" maxlog=3 | ||
@assert method.max_distance > 0 "`max_distance` should be positive and nonzero! Found $(method.max_distance)." | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The same assert is applied again above |
||
segmentize_function = Base.Fix1(_segmentize, method) | ||
return apply(segmentize_function, TraitTarget(GI.LinearRingTrait(), GI.LineStringTrait()), geom; threaded) | ||
new_method = method isa LinearSegments ? Planar() : Geodesic() | ||
segmentize(new_method, geom; max_distance = method.max_distance, threaded) | ||
end | ||
|
||
_segmentize(method, geom) = _segmentize(method, geom, GI.trait(geom)) | ||
#= | ||
This is a method which performs the common functionality for both linear and geodesic algorithms, | ||
and calls out to the "kernel" function which we've defined per linesegment. | ||
=# | ||
function _segmentize(method::Union{LinearSegments, GeodesicSegments}, geom, T::Union{GI.LineStringTrait, GI.LinearRingTrait}) | ||
function _segmentize(method::Union{Planar, Spherical}, geom, T::Union{GI.LineStringTrait, GI.LinearRingTrait}; max_distance) | ||
first_coord = GI.getpoint(geom, 1) | ||
x1, y1 = GI.x(first_coord), GI.y(first_coord) | ||
new_coords = NTuple{2, Float64}[] | ||
sizehint!(new_coords, GI.npoint(geom)) | ||
push!(new_coords, (x1, y1)) | ||
for coord in Iterators.drop(GI.getpoint(geom), 1) | ||
x2, y2 = GI.x(coord), GI.y(coord) | ||
_fill_linear_kernel!(method, new_coords, x1, y1, x2, y2) | ||
_fill_linear_kernel!(method, new_coords, x1, y1, x2, y2; max_distance) | ||
x1, y1 = x2, y2 | ||
end | ||
return rebuild(geom, new_coords) | ||
end | ||
|
||
function _fill_linear_kernel!(method::LinearSegments, new_coords::Vector, x1, y1, x2, y2) | ||
function _fill_linear_kernel!(::Planar, new_coords::Vector, x1, y1, x2, y2; max_distance) | ||
dx, dy = x2 - x1, y2 - y1 | ||
distance = hypot(dx, dy) # this is a more stable way to compute the Euclidean distance | ||
if distance > method.max_distance | ||
n_segments = ceil(Int, distance / method.max_distance) | ||
if distance > max_distance | ||
n_segments = ceil(Int, distance / max_distance) | ||
for i in 1:(n_segments - 1) | ||
t = i / n_segments | ||
push!(new_coords, (x1 + t * dx, y1 + t * dy)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make global named constants for all the numbers in these files, and both insert them in docs and use them for keywords?
Maybe in a different PR, just commenting because this text is added here