Tests for NAs returned when X/Y coordinates are nan

DESCRIPTION

@@ -25,7 +25,7 @@ License: Apache License (== 2.0)
 Encoding: UTF-8
 LazyData: true
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.3.2
+RoxygenNote: 7.3.3
 SystemRequirements: cmake, OpenSSL >= 1.0.1, Abseil >= 20230802.0
 LinkingTo:
     Rcpp,

src/s2-lnglat.cpp

@@ -19,9 +19,14 @@ List s2_lnglat_from_s2_point(List s2_point) {
 
   S2LatLng item;
   for (R_xlen_t i = 0; i < n; i++) {
-    item = S2LatLng(S2Point(x[i], y[i], z[i]));
-    lng[i] = item.lng().degrees();
-    lat[i] = item.lat().degrees();
+    if (std::isnan(x[i]) || std::isnan(y[i]) || std::isnan(z[i])) {
+      lng[i] = NA_REAL;
+      lat[i] = NA_REAL;
+    } else {
+      item = S2LatLng(S2Point(x[i], y[i], z[i]));
+      lng[i] = item.lng().degrees();
+      lat[i] = item.lat().degrees();
+    }
   }
 
   return List::create(_["x"] = lng, _["y"] = lat);
@@ -40,10 +45,16 @@ List s2_point_from_s2_lnglat(List s2_lnglat) {
 
   S2Point item;
   for (R_xlen_t i = 0; i < n; i++) {
-    item = S2LatLng::FromDegrees(lat[i], lng[i]).Normalized().ToPoint();
-    x[i] = item.x();
-    y[i] = item.y();
-    z[i] = item.z();
+    if (std::isnan(lng[i]) || std::isnan(lat[i])) {
+      x[i] = NA_REAL;
+      y[i] = NA_REAL;
+      z[i] = NA_REAL;
+    } else {
+      item = S2LatLng::FromDegrees(lat[i], lng[i]).Normalized().ToPoint();
+      x[i] = item.x();
+      y[i] = item.y();
+      z[i] = item.z();
+    }
   }
 
   return List::create(_["x"] = x, _["y"] = y, _["z"] = z);

src/s2geography/constructor.h

@@ -87,6 +87,10 @@ class Constructor : public Handler {
       }
     } else {
       for (const auto& pt: input_points_) {
+        if (std::isnan(pt.x()) || std::isnan(pt.y())) {
+          throw Exception("Can't unproject point with nan to S2Point");
+        }
+
         points_.push_back(options_.projection()->Unproject(R2Point(pt.x(), pt.y())));
       }
     }
@@ -142,12 +146,12 @@ class PointConstructor : public Constructor {
  private:
   bool coord_empty(const double* coord, int32_t coord_size) {
     for (int32_t i = 0; i < coord_size; i++) {
-      if (!std::isnan(coord[i])) {
-        return false;
+      if (std::isnan(coord[i])) {
+        return true;
       }
     }
 
-    return true;
+    return false;
   }
 };
 

tests/testthat/test-s2-accessors.R

@@ -203,6 +203,12 @@ test_that("s2_distance works", {
   expect_identical(s2_distance(NA_character_, "POINT (0 0)"), NA_real_)
   expect_identical(s2_distance("POINT (0 0)", "POINT EMPTY"), NA_real_)
   expect_identical(s2_distance("POINT EMPTY", "POINT (0 0)"), NA_real_)
+
+  expect_identical(s2_distance("POINT (0 nan)", "POINT (0 0)"), NA_real_)
+  expect_identical(s2_distance("POINT (0 0)", "POINT (0 nan)"), NA_real_)
+  expect_identical(s2_distance("POINT (nan 0)", "POINT (0 0)"), NA_real_)
+  expect_identical(s2_distance("POINT (0 0)", "POINT (nan 0)"), NA_real_)
+  expect_identical(s2_distance("POINT (nan nan)", "POINT (nan nan)"), NA_real_)
 })
 
 test_that("s2_max_distance works", {

tests/testthat/test-s2-lnglat.R

@@ -12,8 +12,8 @@ test_that("s2_lnglat objects can be created from and converted back to R objects
   )
 
   expect_identical(
-    as_s2_lnglat(s2_point(NaN, NaN, NaN)),
-    s2_lnglat(NaN, NaN)
+    as_s2_lnglat(s2_point(NA, NA, NA)),
+    s2_lnglat(NA, NA)
   )
 })
 

tests/testthat/test-s2-point.R

@@ -12,6 +12,17 @@ test_that("s2_point objects can be created from and converted back to R objects"
   )
 })
 
+test_that("s2_point objects propagate NAs on convert to/from lnglat", {
+  expect_identical(as_s2_point(s2_lnglat(NA, NA)), s2_point(NA, NA, NA))
+  expect_identical(as_s2_point(s2_lnglat(NA, 1)), s2_point(NA, NA, NA))
+  expect_identical(as_s2_point(s2_lnglat(1, NA)), s2_point(NA, NA, NA))
+
+  expect_identical(as_s2_lnglat(s2_point(NA, NA, NA)), s2_lnglat(NA, NA))
+  expect_identical(as_s2_lnglat(s2_point(1, NA, NA)), s2_lnglat(NA, NA))
+  expect_identical(as_s2_lnglat(s2_point(NA, 1, NA)), s2_lnglat(NA, NA))
+  expect_identical(as_s2_lnglat(s2_point(NA, NA, 1)), s2_lnglat(NA, NA))
+})
+
 test_that("s2_point can be imported from s2_geography", {
   expect_equal(
     as_s2_point(as_s2_geography("POINT (-64 45)")),