-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add s2_convex_hull()
, add na.rm
argument to s2_convex_hull_agg()
#163
Conversation
Codecov Report
@@ Coverage Diff @@
## main #163 +/- ##
==========================================
- Coverage 94.78% 94.75% -0.03%
==========================================
Files 42 42
Lines 3200 3220 +20
==========================================
+ Hits 3033 3051 +18
- Misses 167 169 +2
Continue to review full report at Codecov.
|
It's a good point about the convex hull of a segment/point. I don't know enough about the application here to know how important this is...it certainly could be included. I can see how it's difficult/verbose to replicate (my attempt is below): library(s2)
fix <- function(result, original, tolerance = 1e-14) {
simple <- s2_rebuild(
result,
options = s2_options(
snap_radius = tolerance
)
)
simple_boundary <- s2_rebuild(
s2_boundary(result),
options = s2_options(
snap_radius = tolerance,
edge_type = "undirected"
)
)
simple_empty <- s2_is_empty(simple)
simple_boundary_empty <- s2_is_empty(simple_boundary)
simple[simple_boundary_empty] <- original[simple_boundary_empty]
simple[simple_empty & !simple_boundary_empty] <-
original[simple_empty & !simple_boundary_empty]
simple
}
geos::geos_convex_hull("MULTIPOINT (3.6 43.2, 0 0, 3.61 43.21)")
#> <geos_geometry[1]>
#> [1] <POLYGON ((0 0, 3.6 43.2, 3.61 43.21, 0 0))>
s2_convex_hull("MULTIPOINT (3.6 43.2, 0 0, 3.61 43.21)")
#> <s2_geography[1]>
#> [1] <POLYGON ((0 0, 3.61 43.21, 3.6 43.2, 0 0))>
fix(
s2_convex_hull("MULTIPOINT (3.6 43.2, 0 0, 3.61 43.21)"),
"MULTIPOINT (3.6 43.2, 0 0, 3.61 43.21)"
)
#> <s2_geography[1]>
#> [1] <POLYGON ((0 0, 3.61 43.21, 3.6 43.2, 0 0))>
geos::geos_convex_hull("POINT (0 0)")
#> <geos_geometry[1]>
#> [1] <POINT (0 0)>
s2_convex_hull("POINT (0 0)")
#> <s2_geography[1]>
#> [1] <POLYGON ((0 0, -5.72949748e-14 3.03663366e-16, -3.03663366e-16 -5.72949748e-14, 0 0))>
fix(s2_convex_hull("POINT (0 0)"), "POINT (0 0)")
#> <s2_geography[1]>
#> [1] <POINT (0 0)>
geos::geos_convex_hull("LINESTRING (0 0, 3.6 43.2)")
#> <geos_geometry[1]>
#> [1] <LINESTRING (0 0, 3.6 43.2)>
s2_convex_hull("LINESTRING (0 0, 3.6 43.2)")
#> <s2_geography[1]>
#> [1] <POLYGON ((1.51774324 21.6094428, 3.6 43.2, 0 0, 1.51774324 21.6094428))>
fix(s2_convex_hull("LINESTRING (0 0, 3.6 43.2)"), "LINESTRING (0 0, 3.6 43.2)")
#> <s2_geography[1]>
#> [1] <LINESTRING (0 0, 3.6 43.2)> Created on 2022-01-16 by the reprex package (v2.0.1) |
BigQuery does this: SELECT ST_CONVEXHULL(ST_GEOGFROMTEXT("POINT (0 1)"))
SELECT ST_CONVEXHULL(ST_GEOGFROMTEXT("LINESTRING (0 0, 3.6 43.2)"))
|
Merge commit '90fa1280a77c53cb81d71ae51642c2504c7b670e' Conflicts: man/s2_boundary.Rd
I really appreciate your work, and returning a polygon is definitely the most stable option for many usages. |
I'm about to merge this one, but if you do get a working implementation feel free to PR it in! |
This PR adds a feature-wise convex-hull generator based on the implementation by @spiry34 in #151. It also adds
na.rm
, since this is how the other_agg()
functions are implemented. This PR completes #150.@spiry34 you're the most familiar with this code...is there anything I've missed or should have considered? You mentioned that the convex hull for lines is different and I haven't checked this in GEOS. I should probably check this in BigQuery to make sure that this implementation is consistent with that (since that's how I've tested the other functions).
Created on 2022-01-16 by the reprex package (v2.0.1)