From b6de1257530f485259052e82e4dc62ef46ea98ff Mon Sep 17 00:00:00 2001 From: ChrisMuir Date: Sun, 23 Sep 2018 11:56:26 -0500 Subject: [PATCH 1/4] handle NA inputs in 'decode()' --- inst/include/googlePolylines.h | 6 ++- src/RcppExports.cpp | 4 +- src/googlePolylines.cpp | 70 +++++++++++++++++++++------------- tests/testthat/test-Decode.R | 5 +++ 4 files changed, 55 insertions(+), 30 deletions(-) diff --git a/inst/include/googlePolylines.h b/inst/include/googlePolylines.h index 2b5913eda..48cef33be 100644 --- a/inst/include/googlePolylines.h +++ b/inst/include/googlePolylines.h @@ -41,7 +41,11 @@ std::vector split(const std::string &s, char delim); Rcpp::CharacterVector getSfClass(SEXP sf); -Rcpp::DataFrame decode_polyline(std::string encoded, std::string encoded_type); +Rcpp::DataFrame decode_polyline(std::string encoded, std::vector& col_headers); + +std::vector get_col_headers(Rcpp::String sfg_dim); + +Rcpp::DataFrame na_dataframe(std::vector& col_headers); Rcpp::String EncodeNumber(int num); diff --git a/src/RcppExports.cpp b/src/RcppExports.cpp index 6d71ff856..a964cb14c 100644 --- a/src/RcppExports.cpp +++ b/src/RcppExports.cpp @@ -31,13 +31,13 @@ BEGIN_RCPP END_RCPP } // rcpp_decode_polyline -Rcpp::List rcpp_decode_polyline(Rcpp::StringVector encodedStrings, std::string encoded_type); +Rcpp::List rcpp_decode_polyline(Rcpp::StringVector encodedStrings, Rcpp::String encoded_type); RcppExport SEXP _googlePolylines_rcpp_decode_polyline(SEXP encodedStringsSEXP, SEXP encoded_typeSEXP) { BEGIN_RCPP Rcpp::RObject rcpp_result_gen; Rcpp::RNGScope rcpp_rngScope_gen; Rcpp::traits::input_parameter< Rcpp::StringVector >::type encodedStrings(encodedStringsSEXP); - Rcpp::traits::input_parameter< std::string >::type encoded_type(encoded_typeSEXP); + Rcpp::traits::input_parameter< Rcpp::String >::type encoded_type(encoded_typeSEXP); rcpp_result_gen = Rcpp::wrap(rcpp_decode_polyline(encodedStrings, encoded_type)); return rcpp_result_gen; END_RCPP diff --git a/src/googlePolylines.cpp b/src/googlePolylines.cpp index 45ce8185c..eaad9e83a 100644 --- a/src/googlePolylines.cpp +++ b/src/googlePolylines.cpp @@ -13,25 +13,31 @@ Rcpp::List rcpp_decode_polyline_list( Rcpp::List encodedList, std::string attrib size_t n = encodedList.size(); Rcpp::List output(n); Rcpp::CharacterVector sfg_dim; - std::string encoded_type; + std::vector col_headers; for (size_t i = 0; i < n; i++) { Rcpp::StringVector polylines = encodedList[i]; sfg_dim = polylines.attr( attribute ); - encoded_type = as< std::string>( sfg_dim[0] ); + col_headers = get_col_headers(sfg_dim[0]); size_t pn = polylines.size(); Rcpp::List polyline_output(pn); for (size_t j = 0; j < pn; j++ ) { + // If polylines[j] is NA, assign a data frame of NA values + if (Rcpp::StringVector::is_na(polylines[j])) { + polyline_output[j] = na_dataframe(col_headers); + continue; + } + Rcpp::StringVector sv(1); sv[0] = polylines[j]; std::string s = Rcpp::as< std::string >(sv); - polyline_output[j] = decode_polyline(s, encoded_type ); + polyline_output[j] = decode_polyline(s, col_headers ); } output[i] = polyline_output; } @@ -40,16 +46,23 @@ Rcpp::List rcpp_decode_polyline_list( Rcpp::List encodedList, std::string attrib } // [[Rcpp::export]] -Rcpp::List rcpp_decode_polyline(Rcpp::StringVector encodedStrings, std::string encoded_type) { +Rcpp::List rcpp_decode_polyline(Rcpp::StringVector encodedStrings, Rcpp::String encoded_type) { int encodedSize = encodedStrings.size(); Rcpp::List results(encodedSize); + std::vector col_headers = get_col_headers(encoded_type); for(int i = 0; i < encodedSize; i++){ + // If encodedStrings[i] is NA, assign a data frame of NA values + if (Rcpp::StringVector::is_na(encodedStrings[i])) { + results[i] = na_dataframe(col_headers); + continue; + } + std::string encoded = Rcpp::as< std::string >(encodedStrings[i]); - Rcpp::DataFrame decoded = decode_polyline(encoded, encoded_type); + Rcpp::DataFrame decoded = decode_polyline(encoded, col_headers); results[i] = decoded; } @@ -59,7 +72,7 @@ Rcpp::List rcpp_decode_polyline(Rcpp::StringVector encodedStrings, std::string e // @param type the type of decoded object, coordinates or ZM Attribute -Rcpp::DataFrame decode_polyline(std::string encoded, std::string encoded_type){ +Rcpp::DataFrame decode_polyline(std::string encoded, std::vector& col_headers) { int len = encoded.size(); int index = 0; @@ -96,31 +109,34 @@ Rcpp::DataFrame decode_polyline(std::string encoded, std::string encoded_type){ } //TODO(ZM attributes) + + return Rcpp::DataFrame::create( + Named(col_headers[0]) = pointsLat, + Named(col_headers[1]) = pointsLon + ); +} - - if (encoded_type == "XYZ" ) { - return Rcpp::DataFrame::create( - Named("Z") = pointsLon, - Named("M") = pointsLat - ); - } else if (encoded_type == "XYM") { - return Rcpp::DataFrame::create( - Named("M") = pointsLon, - Named("Z") = pointsLat - ); - } else if (encoded_type == "XYZM" ) { - return Rcpp::DataFrame::create( - Named("Z") = pointsLon, - Named("M") = pointsLat - ); +std::vector get_col_headers(Rcpp::String sfg_dim) { + std::vector out; + if (sfg_dim == "XYZ" || sfg_dim == "XYZM") { + out.push_back("Z"); + out.push_back("M"); + } else if (sfg_dim == "XYM") { + out.push_back("M"); + out.push_back("Z"); + } else { + out.push_back("lat"); + out.push_back("lon"); } - - // putting latitude first - return Rcpp::DataFrame::create( - Named("lat") = pointsLat, - Named("lon") = pointsLon); + return out; +} +Rcpp::DataFrame na_dataframe(std::vector& col_headers) { + return Rcpp::DataFrame::create( + Named(col_headers[0]) = NA_REAL, + Named(col_headers[1]) = NA_REAL + ); } void EncodeNumber(std::ostringstream& os, int num){ diff --git a/tests/testthat/test-Decode.R b/tests/testthat/test-Decode.R index 1a4505ea2..ac14b31f9 100644 --- a/tests/testthat/test-Decode.R +++ b/tests/testthat/test-Decode.R @@ -13,6 +13,11 @@ test_that("decode works", { expect_error(decode(data.frame()),"I don't know how to decode this object") }) +test_that("NA inputs handled properly", { + expect_equal(decode(NA_character_), + list(data.frame("lat" = NA_real_, "lon" = NA_real_))) +}) + # test_that("decoding ZM columns", { # From d82326fe3283a29fb753f4ff5be485cebe169b51 Mon Sep 17 00:00:00 2001 From: ChrisMuir Date: Sun, 23 Sep 2018 16:01:55 -0500 Subject: [PATCH 2/4] refactor 'decode()' Rcpp funcs for speed --- inst/include/googlePolylines.h | 7 +++++-- src/googlePolylines.cpp | 37 ++++++++++++++++++++++++++-------- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/inst/include/googlePolylines.h b/inst/include/googlePolylines.h index 48cef33be..353d28555 100644 --- a/inst/include/googlePolylines.h +++ b/inst/include/googlePolylines.h @@ -41,11 +41,14 @@ std::vector split(const std::string &s, char delim); Rcpp::CharacterVector getSfClass(SEXP sf); -Rcpp::DataFrame decode_polyline(std::string encoded, std::vector& col_headers); +Rcpp::List decode_polyline(std::string encoded, + std::vector& col_headers, + std::vector& pointsLat, + std::vector& pointsLon); std::vector get_col_headers(Rcpp::String sfg_dim); -Rcpp::DataFrame na_dataframe(std::vector& col_headers); +Rcpp::List na_dataframe(std::vector& col_headers); Rcpp::String EncodeNumber(int num); diff --git a/src/googlePolylines.cpp b/src/googlePolylines.cpp index eaad9e83a..ab1c7f6d1 100644 --- a/src/googlePolylines.cpp +++ b/src/googlePolylines.cpp @@ -13,6 +13,8 @@ Rcpp::List rcpp_decode_polyline_list( Rcpp::List encodedList, std::string attrib size_t n = encodedList.size(); Rcpp::List output(n); Rcpp::CharacterVector sfg_dim; + std::vector pointsLat; + std::vector pointsLon; std::vector col_headers; for (size_t i = 0; i < n; i++) { @@ -37,7 +39,7 @@ Rcpp::List rcpp_decode_polyline_list( Rcpp::List encodedList, std::string attrib sv[0] = polylines[j]; std::string s = Rcpp::as< std::string >(sv); - polyline_output[j] = decode_polyline(s, col_headers ); + polyline_output[j] = decode_polyline(s, col_headers, pointsLat, pointsLon); } output[i] = polyline_output; } @@ -50,6 +52,8 @@ Rcpp::List rcpp_decode_polyline(Rcpp::StringVector encodedStrings, Rcpp::String int encodedSize = encodedStrings.size(); Rcpp::List results(encodedSize); + std::vector pointsLat; + std::vector pointsLon; std::vector col_headers = get_col_headers(encoded_type); for(int i = 0; i < encodedSize; i++){ @@ -62,7 +66,7 @@ Rcpp::List rcpp_decode_polyline(Rcpp::StringVector encodedStrings, Rcpp::String std::string encoded = Rcpp::as< std::string >(encodedStrings[i]); - Rcpp::DataFrame decoded = decode_polyline(encoded, col_headers); + Rcpp::List decoded = decode_polyline(encoded, col_headers, pointsLat, pointsLon); results[i] = decoded; } @@ -72,15 +76,18 @@ Rcpp::List rcpp_decode_polyline(Rcpp::StringVector encodedStrings, Rcpp::String // @param type the type of decoded object, coordinates or ZM Attribute -Rcpp::DataFrame decode_polyline(std::string encoded, std::vector& col_headers) { +Rcpp::List decode_polyline(std::string encoded, + std::vector& col_headers, + std::vector& pointsLat, + std::vector& pointsLon) { int len = encoded.size(); int index = 0; float lat = 0; float lng = 0; - Rcpp::NumericVector pointsLat; - Rcpp::NumericVector pointsLon; + pointsLat.clear(); + pointsLon.clear(); while (index < len){ char b; @@ -110,10 +117,17 @@ Rcpp::DataFrame decode_polyline(std::string encoded, std::vector& c //TODO(ZM attributes) - return Rcpp::DataFrame::create( + // Create List output that has the necessary attributes to make it a + // data.frame object. + Rcpp::List out = Rcpp::List::create( Named(col_headers[0]) = pointsLat, Named(col_headers[1]) = pointsLon ); + + out.attr("class") = "data.frame"; + out.attr("row.names") = seq(1, pointsLat.size()); + + return out; } std::vector get_col_headers(Rcpp::String sfg_dim) { @@ -132,11 +146,18 @@ std::vector get_col_headers(Rcpp::String sfg_dim) { return out; } -Rcpp::DataFrame na_dataframe(std::vector& col_headers) { - return Rcpp::DataFrame::create( +Rcpp::List na_dataframe(std::vector& col_headers) { + // Create List output that has the necessary attributes to make it a + // data.frame object. + Rcpp::List out = Rcpp::List::create( Named(col_headers[0]) = NA_REAL, Named(col_headers[1]) = NA_REAL ); + + out.attr("class") = "data.frame"; + out.attr("row.names") = 1; + + return out; } void EncodeNumber(std::ostringstream& os, int num){ From 1330601da73cf7eb86c17d062e9c26b575d27336 Mon Sep 17 00:00:00 2001 From: ChrisMuir Date: Sun, 23 Sep 2018 17:07:27 -0500 Subject: [PATCH 3/4] updates to reflect function definitions in 'googlePolylines.cpp' --- inst/include/googlePolylines.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/inst/include/googlePolylines.h b/inst/include/googlePolylines.h index 353d28555..6d0058daa 100644 --- a/inst/include/googlePolylines.h +++ b/inst/include/googlePolylines.h @@ -50,9 +50,9 @@ std::vector get_col_headers(Rcpp::String sfg_dim); Rcpp::List na_dataframe(std::vector& col_headers); -Rcpp::String EncodeNumber(int num); +void EncodeNumber(std::ostringstream& os, int num); -Rcpp::String EncodeSignedNumber(int num); +void EncodeSignedNumber(std::ostringstream& os, int num); Rcpp::String encode_polyline(Rcpp::NumericVector latitude, Rcpp::NumericVector longitude); From 782e40987e6ee6154f741ed666d86f5dc5bea429 Mon Sep 17 00:00:00 2001 From: ChrisMuir Date: Mon, 24 Sep 2018 11:45:34 -0500 Subject: [PATCH 4/4] comment out unused variables --- src/encode.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/encode.cpp b/src/encode.cpp index 0948f4589..c7f78a616 100644 --- a/src/encode.cpp +++ b/src/encode.cpp @@ -78,7 +78,7 @@ void make_type(const char *cls, int *tp = NULL, else if (strcmp(cls, "GEOMETRY") == 0) type = SF_Geometry; else if (strcmp(cls, "GEOMETRYCOLLECTION") == 0) - type = SF_GeometryCollection; + type = SF_GeometryCollection; else type = SF_Unknown; if (tp != NULL) @@ -135,8 +135,8 @@ void encode_points( std::ostringstream& os, std::ostringstream& oszm, Rcpp::Nume Rcpp::NumericVector pointLon; Rcpp::NumericVector pointLat; - Rcpp::NumericVector elev(1); - Rcpp::NumericVector meas(1); + //Rcpp::NumericVector elev(1); + //Rcpp::NumericVector meas(1); for (int i = 0; i < n; i++){ pointLon = point(i, 0);