diff --git a/DESCRIPTION b/DESCRIPTION index 61c77f9f..ab3c4720 100644 --- a/DESCRIPTION +++ b/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, diff --git a/src/s2-lnglat.cpp b/src/s2-lnglat.cpp index 72c1f94c..363e2538 100644 --- a/src/s2-lnglat.cpp +++ b/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); diff --git a/src/s2geography/constructor.h b/src/s2geography/constructor.h index 8cd154db..c73a45e8 100644 --- a/src/s2geography/constructor.h +++ b/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; } }; diff --git a/tests/testthat/test-s2-accessors.R b/tests/testthat/test-s2-accessors.R index 19653c54..527bcd7c 100644 --- a/tests/testthat/test-s2-accessors.R +++ b/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", { diff --git a/tests/testthat/test-s2-lnglat.R b/tests/testthat/test-s2-lnglat.R index d22aa135..348cf4bf 100644 --- a/tests/testthat/test-s2-lnglat.R +++ b/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) ) }) diff --git a/tests/testthat/test-s2-point.R b/tests/testthat/test-s2-point.R index 14b33554..d6bd4025 100644 --- a/tests/testthat/test-s2-point.R +++ b/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)")),