Merge pull request #14 from SymbolixAU/factors

factors as strings for #13
SymbolixAU · Apr 23, 2018 · fe109c9 · fe109c9
2 parents 0a7b140 + b2dc0ed
commit fe109c9
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 80 deletions.
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -1,7 +1,7 @@
 Package: geojsonsf
 Type: Package
 Title: GeoJSON to Simple Feature Converter
-Version: 0.3.0008
+Version: 0.3.0009
 Authors@R: c(
     person("David", "Cooley", ,"[email protected]", role = c("aut", "cre"))
     )

diff --git a/src/sf_geojson.cpp b/src/sf_geojson.cpp
@@ -36,20 +36,24 @@ void get_column_type(Rcpp::List& sf,
 		Rcpp::String col = property_names[i];
 		SEXP vec = sf[col];
 
-		switch(TYPEOF(vec)) {
-		case REALSXP:
-			column_types[i] = "Number";
-			break;
-		case INTSXP:
-			column_types[i] = "Number";
-			break;
-		case LGLSXP:
-			column_types[i] = "Logical";
-			break;
-		default: {
-				column_types[i] = "String";
+		if (Rf_isFactor(vec)) {
+			column_types[i] = "String";
+		} else {
+			switch(TYPEOF(vec)) {
+			case REALSXP:
+				column_types[i] = "Number";
 				break;
-		}
+			case INTSXP:
+				column_types[i] = "Number";
+				break;
+			case LGLSXP:
+				column_types[i] = "Logical";
+				break;
+			default: {
+					column_types[i] = "String";
+					break;
+			}
+			}
 		}
 	}
 }

diff --git a/tests/testthat/test-geojson_properties.R b/tests/testthat/test-geojson_properties.R
@@ -18,41 +18,22 @@ test_that("properties captured correctly", {
 	expect_true(sf$name == "foo")
 	expect_true(wkt$name == "foo")
 
-	js <- '[
-	{
+	js <- '[{
 	  "type": "Feature",
 	  "properties" : {},
 	  "geometry": {
 	    "type": "Polygon",
 	    "coordinates": [
-	      [
-	        [-10.0, -10.0],
-	        [10.0, -10.0],
-	        [10.0, 10.0],
-	        [-10.0, -10.0]
-	      ]
-	    ]
-	  }
-	},
-	{
-	  "type": "Feature",
-	  "properties" : { "id" : 1 },
+	      [[-10.0, -10.0],[10.0, -10.0],[10.0, 10.0],[-10.0, -10.0]]]
+      }
+	  },
+	  {
+      "type": "Feature",
+	    "properties" : {"id":1},
 	  "geometry": {
 	    "type": "MultiPolygon",
-	    "coordinates": [
-	      [
-	        [
-	          [180.0, 40.0], [180.0, 50.0], [170.0, 50.0],
-	          [170.0, 40.0], [180.0, 40.0]
-	        ]
-	      ],
-	      [
-	        [
-	          [-170.0, 40.0], [-170.0, 50.0], [-180.0, 50.0],
-	          [-180.0, 40.0], [-170.0, 40.0]
-	        ]
-	      ]
-	    ]
+	    "coordinates": [[[[180.0, 40.0], [180.0, 50.0], [170.0, 50.0],[170.0, 40.0], [180.0, 40.0]]],
+	      [[[-170.0, 40.0], [-170.0, 50.0], [-180.0, 50.0],[-180.0, 40.0], [-170.0, 40.0]]]]
 	  }
 	},
 	{
@@ -61,59 +42,27 @@ test_that("properties captured correctly", {
 	    {
 	      "type": "Feature",
 	      "properties": {"id" : 2, "value" : "foo"},
-	      "geometry": {
-	        "type": "Point",
-	        "coordinates": [100.0, 0.0]
-	      }
+	      "geometry": {"type": "Point","coordinates": [100.0, 0.0]}
 	    },
 	    {
 	      "type": "Feature",
 	      "properties": null,
 	      "geometry": {
 	        "type": "LineString",
-	        "coordinates": [
-	          [101.0, 0.0],
-	          [102.0, 1.0]
-	        ]
+	        "coordinates": [[101.0, 0.0],[102.0, 1.0]]
 	      }
 	    }
 	  ]
 	},
 	{
 	  "type": "GeometryCollection",
 	  "geometries": [
-	    {
-	      "type": "Point",
-	      "coordinates": [100.0, 0.0]
-	    },
-	    {
-	      "type": "LineString",
-	      "coordinates": [
-	        [101.0, 0.0],
-	        [102.0, 1.0]
-	      ]
-	    },
-	    {
-	      "type" : "MultiPoint",
-	      "coordinates" : [
-	        [0,0],
-	        [1,1],
-	        [2,2]
-	      ]
-	    }
+	    {"type": "Point","coordinates": [100.0, 0.0]},
+	    {"type": "LineString","coordinates": [[101.0, 0.0],[102.0, 1.0]]},
+	    {"type" : "MultiPoint","coordinates" : [[0,0],[1,1],[2,2]]}
 	  ]
 	},
-	{
-	"type": "Polygon",
-	    "coordinates": [
-	      [
-	        [-10.0, -10.0],
-	        [10.0, -10.0],
-	        [10.0, 10.0],
-	        [-10.0, -10.0]
-	      ]
-	    ]
-	}
+	{"type": "Polygon","coordinates": [[[-10.0, -10.0],[10.0, -10.0],[10.0, 10.0],[-10.0, -10.0]]]}
 	]'
 	sf <- geojson_sf(js)
 	wkt <- geojson_wkt(js)
@@ -180,13 +129,28 @@ test_that("null geometries are valid for features", {
 	{"type":"Feature","properties":{"id":3},"geometry":null}
 	]}'
 	expect_true(nrow(geojson_sf(js)) == 3)
+	expect_true(all(geojson_sf(js)[['id']] == 1:3))
+	expect_true(is.null(geojson_sf(js)[3, 'geometry'][[1]][[1]][1][[1]]))
+	## TODO: Which geometry should this be?
+
+
+	js <- '{"type":"FeatureCollection","features":[
+	{"type":"Feature","properties":{"id":3},"geometry":{"type":"MultiPoint","coordinates":[[0,0],[1,1]]}},
+	{"type":"Feature","properties":{"id":1},"geometry":null},
+	{"type":"Feature","properties":{"id":2},"geometry":{"type":"Point","coordinates":[0,0]}}
+	]}'
+	expect_true(nrow(geojson_sf(js)) == 3)
+	expect_true(all(geojson_sf(js)[['id']] == c(3,1,2)))
+	expect_true(is.null(geojson_sf(js)[2, 'geometry'][[1]][[1]][1][[1]]))
 	## 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")
 
 
+
 })
 
 

diff --git a/tests/testthat/test-sf_geojson.R b/tests/testthat/test-sf_geojson.R
@@ -259,3 +259,23 @@ test_that("errors are handled", {
 
 
 })
+
+test_that("factors are strings", {
+
+	skip_on_cran()
+	skip_on_travis()
+
+	fgc <- '
+	{"type": "Feature","geometry": {"type": "GeometryCollection","geometries": [
+	{"type": "Point","coordinates": [100.0, 0.0]},
+	{"type": "LineString","coordinates": [[101.0, 0.0], [102.0, 1.0]]}]},
+	"properties": {"prop0": "value0","prop1": "value1"}}'
+
+	sf <- geojson_sf(fgc)
+	sf$prop0 <- as.factor(sf$prop0)
+
+	expect_true(jsonlite::validate(sf_geojson(sf)))
+
+})
+
+