From 51c0d8017be3ec947bc5b13b6e7e5e7cb05da26c Mon Sep 17 00:00:00 2001 From: David Cooley Date: Mon, 23 Apr 2018 15:55:48 +1000 Subject: [PATCH 1/3] not bothering with strict for #11 --- R/scratch.R | 70 +++++++++++++++++++++++++++++++++++++------ src/geojson_sfc.cpp | 5 ++++ src/geojson_to_sf.cpp | 36 +++++++++++++--------- 3 files changed, 88 insertions(+), 23 deletions(-) diff --git a/R/scratch.R b/R/scratch.R index 561c1eb..0d818db 100644 --- a/R/scratch.R +++ b/R/scratch.R @@ -1,15 +1,67 @@ ## TODO: -## work on sfc objects -# sf <- sf::st_sfc(list(sf::st_point(c(0,0)), sf::st_point(c(1,1)))) -# attributes(sf) -# sfc_geojson(sf) -## an sfc object doesn't have properties. -## and it doesn't have an 'sf_column' attribute +# library(Rcpp) +# +# cppFunction('Rcpp::NumericVector vec() { +# Rcpp::NumericVector point(2); +# point[0] = NA_REAL; +# return point; +# }') +# +# vec() +# +# +# js <- '{"type":"Point","coordinates":[null,null]}' +# sf::st_read(js) +# +# js <- '{"type":"Point","coordinates":[,]}' +# sf::st_read(js) +# +# js <- '{"type":"Point","coordinates":[]}' +# sf::st_read(js) +# +# js <- '{"type":"Point","coordinates":{}}' +# sf::st_read(js) +# +# url <- "https://data.seattle.gov/resource/pdbw-sw7q.geojson" +# sf <- sf::st_read(url, quiet = T) +# +# js <- '{"type","Feature","geometry":null}' +# sf::st_read(js) +# +# js <- '{"type","Feature","geometry":null,"properties":{}}' +# sf::st_read(js) +# +# ## NULL geometry should be fine / parse +# js <- '{"type":"FeatureCollection","features":[ +# {"type":"Feature","properties":{"id":1},"geometry":{"type":"Point","coordinates":[0,0]}}, +# {"type":"Feature","properties":{"id":2},"geometry":null} +# ]}' +# sf <- sf::st_read(js) +# sf +# geojson_sf(js) +# +# js <- '{"type":"Feature","properties":{"id":2},"geometry":null}' +# sf <- sf::st_read(js) +# sf2 <- geojson_sf(js) + +# sf +# sf2 +# +# str(sf) +# str(sf2) +# +# str(sf$geometry) +# str(sf2$geometry) +# +# attr(sf2$geometry, "class") <- c("sfc_POLYGON", "sfc") +# +# str(sf) +# str(sf2) ## the 'id' value is missing +# +# str(sf$geometry) +# str(sf2$geometry) -# sf <- sf::st_sf(geometry = sf::st_sfc(list(sf::st_point(c(0,0)), sf::st_point(c(1,1))))) -# attributes(sf) -# sf_geojson(sf, TRUE) diff --git a/src/geojson_sfc.cpp b/src/geojson_sfc.cpp index 20be19d..7b6d0fd 100644 --- a/src/geojson_sfc.cpp +++ b/src/geojson_sfc.cpp @@ -160,7 +160,12 @@ void fetch_geometries(Rcpp::List& sf, Rcpp::List& res, int& sfg_counter) { break; } default: { + Rcpp::Rcout << "debug: default geometry" << std::endl; + //Rcpp::List tmp; + //tmp.attr("class") = Rcpp::CharacterVector::create("XY","POLYGON","sfg"); Rcpp::stop("Geometry could not be determined"); + //res[sfg_counter] = tmp; + //sfg_counter++; } } } diff --git a/src/geojson_to_sf.cpp b/src/geojson_to_sf.cpp index 152f5fc..87ebf37 100644 --- a/src/geojson_to_sf.cpp +++ b/src/geojson_to_sf.cpp @@ -83,7 +83,7 @@ Rcpp::List parse_geometry_collection_object(const Value& val, validate_type(gcval, sfg_objects); geom_type = gcval["type"].GetString(); parse_geometry_object(geom_collection, i, gcval, bbox, geometry_types, sfg_objects); - } +} geom_collection.attr("class") = sfg_attributes("GEOMETRYCOLLECTION"); return geom_collection; @@ -101,19 +101,7 @@ Rcpp::List parse_feature_object(const Value& feature, validate_geometry(feature, sfg_objects); validate_properties(feature, sfg_objects); - const Value& geometry = feature["geometry"]; - validate_type(geometry, sfg_objects); - std::string type = geometry["type"].GetString(); - Rcpp::List sfc(1); - - if (type == "GeometryCollection") { - sfc[0] = parse_geometry_collection_object(geometry, bbox, geometry_types, sfg_objects); - } else { - parse_geometry_object(sfc, 0, geometry, bbox, geometry_types, sfg_objects); - } - - sfg_objects++; - + // PROPERTIES const Value& p = feature["properties"]; get_property_keys(p, property_keys); get_property_types(p, property_types); @@ -126,6 +114,26 @@ Rcpp::List parse_feature_object(const Value& feature, Value properties(feature["properties"], doc_properties.GetAllocator()); doc_properties.AddMember(n, properties, doc_properties.GetAllocator()); + // GEOMETRY + Rcpp::List sfc(1); + + const Value& geometry = feature["geometry"]; + + Rcpp::Rcout << "debug: geometry size: " << geometry.Size() << std::endl; + //if (geometry.Size() > 0) { + + validate_type(geometry, sfg_objects); + std::string type = geometry["type"].GetString(); + + if (type == "GeometryCollection") { + sfc[0] = parse_geometry_collection_object(geometry, bbox, geometry_types, sfg_objects); + } else { + parse_geometry_object(sfc, 0, geometry, bbox, geometry_types, sfg_objects); + } + //} + + sfg_objects++; + return sfc; } From 92106c03754488daa7d5a6ae8c7be6a6f3c96ed2 Mon Sep 17 00:00:00 2001 From: symbolixau Date: Mon, 23 Apr 2018 17:26:54 +1000 Subject: [PATCH 2/3] parsing null geometry for #11 --- R/scratch.R | 8 ++--- src/geojson_to_sf.cpp | 71 ++++++++++++++++++++++--------------------- 2 files changed, 41 insertions(+), 38 deletions(-) diff --git a/R/scratch.R b/R/scratch.R index 0d818db..e550f0b 100644 --- a/R/scratch.R +++ b/R/scratch.R @@ -34,7 +34,7 @@ # js <- '{"type","Feature","geometry":null,"properties":{}}' # sf::st_read(js) # -# ## NULL geometry should be fine / parse +## NULL geometry should be fine / parse # js <- '{"type":"FeatureCollection","features":[ # {"type":"Feature","properties":{"id":1},"geometry":{"type":"Point","coordinates":[0,0]}}, # {"type":"Feature","properties":{"id":2},"geometry":null} @@ -42,8 +42,9 @@ # sf <- sf::st_read(js) # sf # geojson_sf(js) -# -# js <- '{"type":"Feature","properties":{"id":2},"geometry":null}' + + +# js <- '{"type":"Feature","properties":{"id":2},"geometry": null}' # sf <- sf::st_read(js) # sf2 <- geojson_sf(js) @@ -64,4 +65,3 @@ # str(sf$geometry) # str(sf2$geometry) - diff --git a/src/geojson_to_sf.cpp b/src/geojson_to_sf.cpp index 87ebf37..5084a06 100644 --- a/src/geojson_to_sf.cpp +++ b/src/geojson_to_sf.cpp @@ -83,7 +83,7 @@ Rcpp::List parse_geometry_collection_object(const Value& val, validate_type(gcval, sfg_objects); geom_type = gcval["type"].GetString(); parse_geometry_object(geom_collection, i, gcval, bbox, geometry_types, sfg_objects); -} + } geom_collection.attr("class") = sfg_attributes("GEOMETRYCOLLECTION"); return geom_collection; @@ -98,46 +98,49 @@ Rcpp::List parse_feature_object(const Value& feature, Document& doc_properties, std::map< std::string, std::string>& property_types) { - validate_geometry(feature, sfg_objects); - validate_properties(feature, sfg_objects); - - // PROPERTIES - const Value& p = feature["properties"]; - get_property_keys(p, property_keys); - get_property_types(p, property_types); - - //https://stackoverflow.com/a/33473321/5977215 - std::string s = std::to_string(sfg_objects); - Value n(s.c_str(), doc_properties.GetAllocator()); - - // TODO: is this method deep-cloning? - Value properties(feature["properties"], doc_properties.GetAllocator()); - doc_properties.AddMember(n, properties, doc_properties.GetAllocator()); - - // GEOMETRY - Rcpp::List sfc(1); - - const Value& geometry = feature["geometry"]; + validate_geometry(feature, sfg_objects); + validate_properties(feature, sfg_objects); + + const Value& geometry = feature["geometry"]; + //validate_type(geometry, sfg_objects); + //std::string type = geometry["type"].GetString(); + Rcpp::List sfc(1); + + if (geometry.Size() > 0) { + + validate_type(geometry, sfg_objects); + std::string type = geometry["type"].GetString(); + + if (type == "GeometryCollection") { + sfc[0] = parse_geometry_collection_object(geometry, bbox, geometry_types, sfg_objects); + } else { + parse_geometry_object(sfc, 0, geometry, bbox, geometry_types, sfg_objects); + } + } else { + // needs to be a null geometry + Rcpp::List nullObj; + nullObj.attr("class") = sfg_attributes("POLYGON"); + sfc[0] = nullObj; + geometry_types.insert("POLYGON"); + } - Rcpp::Rcout << "debug: geometry size: " << geometry.Size() << std::endl; - //if (geometry.Size() > 0) { + sfg_objects++; - validate_type(geometry, sfg_objects); - std::string type = geometry["type"].GetString(); + const Value& p = feature["properties"]; + get_property_keys(p, property_keys); + get_property_types(p, property_types); - if (type == "GeometryCollection") { - sfc[0] = parse_geometry_collection_object(geometry, bbox, geometry_types, sfg_objects); - } else { - parse_geometry_object(sfc, 0, geometry, bbox, geometry_types, sfg_objects); - } - //} + //https://stackoverflow.com/a/33473321/5977215 + std::string s = std::to_string(sfg_objects); + Value n(s.c_str(), doc_properties.GetAllocator()); - sfg_objects++; + // TODO: is this method deep-cloning? + Value properties(feature["properties"], doc_properties.GetAllocator()); + doc_properties.AddMember(n, properties, doc_properties.GetAllocator()); - return sfc; + return sfc; } - Rcpp::List parse_feature_collection_object(const Value& fc, Rcpp::NumericVector& bbox, std::set< std::string >& geometry_types, From 765751e69972d7818887febcada179ba534240bd Mon Sep 17 00:00:00 2001 From: symbolixau Date: Mon, 23 Apr 2018 20:34:58 +1000 Subject: [PATCH 3/3] parses null geometries for #11 --- R/scratch.R | 60 +----------------------- src/geojson_to_sf.cpp | 15 +++++- tests/testthat/test-geojson_properties.R | 53 +++++++++++++++++++++ 3 files changed, 69 insertions(+), 59 deletions(-) diff --git a/R/scratch.R b/R/scratch.R index e550f0b..1c9bbf6 100644 --- a/R/scratch.R +++ b/R/scratch.R @@ -2,66 +2,10 @@ ## TODO: -# library(Rcpp) -# -# cppFunction('Rcpp::NumericVector vec() { -# Rcpp::NumericVector point(2); -# point[0] = NA_REAL; -# return point; -# }') -# -# vec() -# -# -# js <- '{"type":"Point","coordinates":[null,null]}' -# sf::st_read(js) -# -# js <- '{"type":"Point","coordinates":[,]}' -# sf::st_read(js) -# -# js <- '{"type":"Point","coordinates":[]}' -# sf::st_read(js) -# -# js <- '{"type":"Point","coordinates":{}}' -# sf::st_read(js) + # # url <- "https://data.seattle.gov/resource/pdbw-sw7q.geojson" # sf <- sf::st_read(url, quiet = T) -# -# js <- '{"type","Feature","geometry":null}' -# sf::st_read(js) -# -# js <- '{"type","Feature","geometry":null,"properties":{}}' -# sf::st_read(js) -# -## NULL geometry should be fine / parse -# js <- '{"type":"FeatureCollection","features":[ -# {"type":"Feature","properties":{"id":1},"geometry":{"type":"Point","coordinates":[0,0]}}, -# {"type":"Feature","properties":{"id":2},"geometry":null} -# ]}' -# sf <- sf::st_read(js) -# sf -# geojson_sf(js) - +# sf2 <- geojson_sf(url) -# js <- '{"type":"Feature","properties":{"id":2},"geometry": null}' -# sf <- sf::st_read(js) -# sf2 <- geojson_sf(js) - -# sf -# sf2 -# -# str(sf) -# str(sf2) -# -# str(sf$geometry) -# str(sf2$geometry) -# -# attr(sf2$geometry, "class") <- c("sfc_POLYGON", "sfc") -# -# str(sf) -# str(sf2) ## the 'id' value is missing -# -# str(sf$geometry) -# str(sf2$geometry) diff --git a/src/geojson_to_sf.cpp b/src/geojson_to_sf.cpp index 5084a06..5828634 100644 --- a/src/geojson_to_sf.cpp +++ b/src/geojson_to_sf.cpp @@ -89,7 +89,6 @@ Rcpp::List parse_geometry_collection_object(const Value& val, return geom_collection; } - Rcpp::List parse_feature_object(const Value& feature, Rcpp::NumericVector& bbox, std::set< std::string >& geometry_types, @@ -117,8 +116,22 @@ Rcpp::List parse_feature_object(const Value& feature, parse_geometry_object(sfc, 0, geometry, bbox, geometry_types, sfg_objects); } } else { + // TODO: + // insert the geometry as per teh rules followed by 'sf' + // // needs to be a null geometry Rcpp::List nullObj; + /* + std::string temp_geom; + if (geometry_types.empty()) { + temp_geom = "POINT"; + } else { + temp_geom = *geometry_types.rbegin(); + transform(temp_geom.begin(), temp_geom.end(), temp_geom.begin(), ::toupper); + } + */ + //Rcpp::Rcout << "debug: temp_geom: " << temp_geom << std::endl; + nullObj.attr("class") = sfg_attributes("POLYGON"); sfc[0] = nullObj; geometry_types.insert("POLYGON"); diff --git a/tests/testthat/test-geojson_properties.R b/tests/testthat/test-geojson_properties.R index c705866..65766cf 100644 --- a/tests/testthat/test-geojson_properties.R +++ b/tests/testthat/test-geojson_properties.R @@ -141,3 +141,56 @@ test_that("sf and sfc created equally", { expect_true(all(class(sf$geometry) == class(sfc))) }) + +test_that("null geometries are valid for features", { + + js <- '{"type":"Point","coordinates":[null,null]}' + geojson_sf(js) ## TODO: needs to error!! + + js <- '{"type":"Point","coordinates":[,]}' + expect_error(geojson_sf(js), "Invalid JSON") + + js <- '{"type":"Point","coordinates":[]}' + expect_error(geojson_sf(js), "Invalid lon/lat object") + + js <- '{"type":"Point","coordinates":{}}' + expect_error(geojson_sf(js), "No 'array' member at object index 0 - invalid GeoJSON") + + js <- '{"type","Feature","geometry":null}' + expect_error(geojson_sf(js), "Invalid JSON") + + js <- '{"type","Feature","geometry":null,"properties":{}}' + expect_error(geojson_sf(js), "Invalid JSON") + + ## NULL geometry should be fine / parse + js <- '{"type":"FeatureCollection","features":[ + {"type":"Feature","properties":{"id":1},"geometry":{"type":"Point","coordinates":[0,0]}}, + {"type":"Feature","properties":{"id":2},"geometry":null} + ]}' + expect_true(nrow(geojson_sf(js)) == 2) + ## TODO: Which geometry should the empty row be? + + js <- '{"type":"Feature","properties":{"id":2},"geometry": null}' + expect_true(nrow(geojson_sf(js)) == 1) + ## TODO: Which geometry should the empty row be? + + js <- '{"type":"FeatureCollection","features":[ + {"type":"Feature","properties":{"id":1},"geometry":{"type":"MultiPoint","coordinates":[[0,0],[1,1]]}}, + {"type":"Feature","properties":{"id":2},"geometry":{"type":"Point","coordinates":[0,0]}}, + {"type":"Feature","properties":{"id":3},"geometry":null} + ]}' + expect_true(nrow(geojson_sf(js)) == 3) + ## TODO: Which geometry should this be? + + ## null geometries that aren't part of features should still error + js <- '{"type":"Point","coordinates":null}' + expect_error(geojson_sf(js), "No 'array' member at object index 0 - invalid GeoJSON") + + +}) + + + + + +