Merge pull request #121 from r-spatial/dewey-dev

make sure aggregate centroids normalize points
r-spatial · Jun 5, 2021 · 8491cd3 · 8491cd3
2 parents e39716e + 8401275
commit 8491cd3
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 1 deletion.
diff --git a/src/s2-transformers.cpp b/src/s2-transformers.cpp
@@ -325,7 +325,7 @@ List cpp_s2_centroid_agg(List geog, bool naRm) {
   if (cumCentroid.Norm2() == 0) {
     output[0] = Rcpp::XPtr<Geography>(new PointGeography());
   } else {
-    output[0] = Rcpp::XPtr<Geography>(new PointGeography(cumCentroid));
+    output[0] = Rcpp::XPtr<Geography>(new PointGeography(cumCentroid.Normalize()));
   }
 
   return output;

diff --git a/tests/testthat/test-s2-transformers.R b/tests/testthat/test-s2-transformers.R
@@ -14,6 +14,24 @@ test_that("s2_centroid() works", {
   )
 })
 
+test_that("s2_centroid() and s2_centroid_agg() normalize points", {
+  expect_equal(
+    s2_distance(
+      s2_centroid("MULTIPOINT (1 1, 1 1)"),
+      "POINT (1 1)"
+    ),
+    0
+  )
+
+  expect_equal(
+    s2_distance(
+      s2_centroid_agg(c("POINT (1 1)", "POINT (1 1)")),
+      "POINT (1 1)"
+    ),
+    0
+  )
+})
+
 test_that("s2_boundary() works", {
   expect_true(s2_is_empty(s2_boundary("POINT (30 10)")))
   expect_true(s2_is_empty(s2_boundary("POINT EMPTY")))