geojsonio::geojson_json call leads to Error: protect(): protection stack overflow

[sheffe]

Crosspost at @ateucher 's request from rmapshaper#71 here

I'm working with a moderately sized sf polygon object (all US census tracts downloaded by the tigris package), about 73k rows and 150Mb in memory according to object.size(). I'm using rmapshaper::ms_simplify() with default params, and getting a consistent error of the form Error: protect(): protection stack overflow. Other sf polygon objects work just fine, but they're meaningfully smaller. I'm running MacOS with 64GB RAM and system monitors show relatively low memory usage during runtime. I've tried using rmapshaper::ms_simplify() on the sf object and a conversion with geojsonio::geojson_json() and geojsonio::geojson_list(), all to the same effect.

Andy writes:

It looks like the error occurs in geojsonio::geojson_json (which is used internally in rmapshaper. From your example, if you run just geojsonio::geojson_json(alltracts) you get the same error. In particular, if you look at the output of traceback() after the error it looks like it is occurring with jqr... I'll post more detail tomorrow once I dig into it a bit more.

This gist has code with reproducible example and devtools::session_info().

[sckott]

oof, sorry about that @sheffe

I ran the examle, and indeed get the same thing.

looking into it.

[ateucher]

@sckott I also noticed that converting big sf/sp objects to geojson has slowed down since the addition of adding classes via geojson. I’ll post a reprex soon... but wonder if we can somehow provide an ‘escape’ of some sort to avoid it?

[sckott]

thanks @ateucher - sounds good.

wonder if we can somehow provide an ‘escape’ of some sort to avoid it?

possibly, will have to make sure it wouldn't break behavior.

[ateucher]

I don't know enough about C++ to know whether this is related to the protect/stackoverflow issues, but here is a test of timings with a largish sf object, converting sf to geojson with/without adding the classes via geojson:

library(bcmaps)
library(geojsonio)

bc <- bc_bound_hres()

system.time(geojson_json(bc))
#>    user  system elapsed 
#>  14.949   1.864  29.755

# View source
geojsonio:::geojson_json.sf
#> function (input, lat = NULL, lon = NULL, group = NULL, geometry = "point", 
#>     type = "auto", convert_wgs84 = FALSE, crs = NULL, ...) 
#> {
#>     geoclass(as.json(geojson_list(input, convert_wgs84 = convert_wgs84, 
#>         crs = crs), ...), type)
#> }
#> <environment: namespace:geojsonio>

# We can remove the `geoclass` function to see how fast it is without that:
system.time(as.json(geojson_list(bc)))
#>    user  system elapsed 
#>   3.327   0.145   5.866

[ateucher]

@sckott as a way around it, could we either:

1. Add an argument character_only (boolean, default FALSE to preserve current behaviour) to geojson_json() to force skipping of geoclass

2. Add a "character" option to the type argument (in addition to current "auto", "FeatureCollection", "Point", "Polygon" etc) that would do the same?

[ateucher]

If option 2, the geoclass function could look like this:

geoclass <- function(x, type = "FeatureCollection") {
  res <- switch(type,
    "auto" = geojson::to_geojson(unclass(x)),
    "Point" = geojson::point(unclass(x)),
    "LineString" = geojson::linestring(unclass(x)),
    "Polygon" = geojson::polygon(unclass(x)),
    "MultiPoint" = geojson::multipoint(unclass(x)),
    "MultiLineString" = geojson::multilinestring(unclass(x)),
    "MultiPolygon" = geojson::multipolygon(unclass(x)),
    "Feature" = geojson::feature(unclass(x)),
    "FeatureCollection" = geojson::featurecollection(unclass(x)),
    "GeometryCollection" = geojson::geometrycollection(unclass(x)), 
    "character" = unclass(x) # <---- New option to skip the geojsonifying...
  )
  class(res) <- c(class(res), c("geo_json", "json"))
  return(res)
}

[sckott]

I guess a character option in the type parameter doesn't fit in thematically because all the options in that param are all for the type of geojson object, and not data type (character, numeric, etc.) Maybe use a word like "skip" to indicate skipping the coercion to a geojson pkg class?

[sckott]

install from remotes::install_github("ropensci/geojsonio@skip-geoclass")

library(bcmaps)
library(geojsonio)
bc <- bc_bound_hres()
system.time(geojson_json(bc, type = "skip"))
#>   user  system elapsed
#>  1.680   0.093   1.787
system.time(geojson_json(bc))
#>   user  system elapsed
#>  13.259   1.154  15.823

[sckott]

thoughts?

[ateucher]

Makes sense to me. Thanks @sckott !

[ateucher]

@sckott should we add this to the v0.6.0 milestone?

[sckott]

@ateucher yep, sounds good, can you do that? or do you not have access?

[ateucher]

Done!

[sckott]

@ateucher is this now working in rmapshaper

[ateucher]

I’ll check. I had actually circumvented it, but will likely go back to this

[sckott]

should we leave this open or close?

[sheffe]

Before closing - thanks, everyone!

[ateucher]

Go ahead and close. It’s a rmapshaper issue now ;)

[github-actions]

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.