ropensci / geojsonio

Convert many data formats to & from GeoJSON & TopoJSON
https://docs.ropensci.org/geojsonio
Other
150 stars 59 forks source link

Remove class from columns of class 'unit' #107

Closed ateucher closed 7 years ago

ateucher commented 7 years ago

When sf calculates the area or length of a feature, it gives the resulting object the 'units' class (from the units package). In geoojsonio::to_json (here) we should detect these and as.numeric them before converting to json. Or we could set force = TRUE in jsonlite::toJSON ?

library(sf)
#> Linking to GEOS 3.5.0, GDAL 2.1.1, proj.4 4.9.3
library(geojsonio)
#> 
#> Attaching package: 'geojsonio'
#> The following object is masked from 'package:base':
#> 
#>     pretty

file <- system.file("examples", "california.geojson", package = "geojsonio")
sf_obj <- read_sf(file)

## No problem
foo <- geojson_json(sf_obj)

## Creates an area column of class 'units'
sf_obj$area <- st_area(sf_obj)

class(sf_obj$area)
#> [1] "units"

geojson_json(sf_obj)
#> Error: No method asJSON S3 class: units
Session info ``` r devtools::session_info() #> Session info ------------------------------------------------------------- #> setting value #> version R version 3.4.0 (2017-04-21) #> system x86_64, mingw32 #> ui RTerm #> language (EN) #> collate English_Canada.1252 #> tz America/Los_Angeles #> date 2017-05-29 #> Packages ----------------------------------------------------------------- #> package * version date source #> backports 1.1.0 2017-05-22 CRAN (R 3.4.0) #> base * 3.4.0 2017-04-21 local #> compiler 3.4.0 2017-04-21 local #> curl 2.6 2017-04-27 CRAN (R 3.4.0) #> datasets * 3.4.0 2017-04-21 local #> DBI 0.6-1 2017-04-01 CRAN (R 3.4.0) #> devtools 1.13.1 2017-05-13 CRAN (R 3.4.0) #> digest 0.6.12 2017-01-27 CRAN (R 3.4.0) #> evaluate 0.10 2016-10-11 CRAN (R 3.4.0) #> foreign 0.8-68 2017-04-24 CRAN (R 3.4.0) #> geojsonio * 0.3.2 2017-02-06 CRAN (R 3.4.0) #> geosphere 1.5-5 2016-06-15 CRAN (R 3.4.0) #> graphics * 3.4.0 2017-04-21 local #> grDevices * 3.4.0 2017-04-21 local #> grid 3.4.0 2017-04-21 local #> htmltools 0.3.6 2017-04-28 CRAN (R 3.4.0) #> httr 1.2.1 2016-07-03 CRAN (R 3.4.0) #> jsonlite 1.4 2017-04-08 CRAN (R 3.4.0) #> knitr 1.16 2017-05-18 CRAN (R 3.4.0) #> lattice 0.20-35 2017-03-25 CRAN (R 3.4.0) #> magrittr 1.5 2014-11-22 CRAN (R 3.4.0) #> maptools 0.9-2 2017-03-25 CRAN (R 3.4.0) #> memoise 1.1.0 2017-04-21 CRAN (R 3.4.0) #> methods * 3.4.0 2017-04-21 local #> R6 2.2.1 2017-05-10 CRAN (R 3.4.0) #> Rcpp 0.12.11 2017-05-22 CRAN (R 3.4.0) #> rgdal 1.2-7 2017-04-25 CRAN (R 3.4.0) #> rgeos 0.3-23 2017-04-06 CRAN (R 3.4.0) #> rmarkdown 1.5.9000 2017-05-16 Github (rstudio/rmarkdown@0f89945) #> rprojroot 1.2 2017-01-16 CRAN (R 3.4.0) #> sf * 0.4-3 2017-05-15 CRAN (R 3.4.0) #> sp 1.2-4 2016-12-22 CRAN (R 3.4.0) #> stats * 3.4.0 2017-04-21 local #> stringi 1.1.5 2017-04-07 CRAN (R 3.4.0) #> stringr 1.2.0 2017-02-18 CRAN (R 3.4.0) #> tools 3.4.0 2017-04-21 local #> udunits2 0.13 2016-11-17 CRAN (R 3.4.0) #> units 0.4-4 2017-04-20 CRAN (R 3.4.0) #> utils * 3.4.0 2017-04-21 local #> V8 1.5 2017-04-25 CRAN (R 3.4.0) #> withr 1.0.2 2016-06-20 CRAN (R 3.4.0) #> yaml 2.1.14 2016-11-12 CRAN (R 3.4.0) ```
ateucher commented 7 years ago

I lean towards the latter for simplicity, and would deal with other special classes that come along. I can't think of a reason off the top of my head what the downsides would be... thoughts @sckott?

sckott commented 7 years ago

Agree forcing sounds good!

ateucher commented 7 years ago

Closed in a91a7b9d8faeccc12ca8ecf3d6e09f3fc6b6ccc5

ateucher commented 7 years ago

@sckott now it's really done I think, if you want to have a look. writeOGR doesn't like special classes either, so I tackled that in https://github.com/ropensci/geojsonio/commit/ba495356b219c4b92f5b97afcfe4bb9115b4793b

sckott commented 7 years ago

LGTM

mdsumner commented 7 years ago

Might be worth putting that classes logic into rgdal itself, I'll have a look. Roger patched the dev version recently to accept a tibble (it was otherwise relying on drop = TRUE).

ateucher commented 7 years ago

Thanks @mdsumner. It is explicitly documented in the help page for writeOGR that anything other than those basic classes will throw an error, so it seems they wanted the user to decide what to do with special classes

mdsumner commented 7 years ago

That's a good point, but I think the opportunities for this to be a problem are much more frequent now that the perspectives are changing. :)

ateucher commented 7 years ago

That's probably true! I certainly have no issues if you want to pursue it 😄

github-actions[bot] commented 2 years ago

This issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.