Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hard session/process shutdown when reading empty geojson #58

Closed
sheffe opened this issue Dec 15, 2018 · 4 comments
Closed

Hard session/process shutdown when reading empty geojson #58

sheffe opened this issue Dec 15, 2018 · 4 comments
Assignees
Labels
bug
Milestone

Comments

@sheffe
Copy link

@sheffe sheffe commented Dec 15, 2018

Love this package, thanks for putting it out there!

I'm working through a process using osmium that clips OSM data by a polygon boundary and converts the subsetted data to geojson. Sometimes, if there's no data within that boundary, an empty GeoJSON is written. It looks like you've solved some null objects elsewhere, as in #36, but this particular example still breaks for me.

Here's the reprex:

library(geojsonsf)
library(sf)

empty_geojson <- "{\"type\":\"FeatureCollection\",\"features\":[\n\n]}\n"

writeLines(empty_geojson, con = "empty_geojson_example.geojson")

# This successfully reads as 0 feature/0 field sf object
st_read("empty_geojson_example.geojson")

# This causes hard shutdown/session crash on my system
geojson_sf("empty_geojson_example.geojson")

(If it's useful to know this, the empty_geojson example is the exact text content of the file created by osmium. It seems to be valid geojson to me, and sf can read it, but I'm not completely sure of that belief.)

and here's my devtools::session_info()

─ Session info
setting value
version R version 3.5.1 (2018-07-02)
os macOS 10.14.2
system x86_64, darwin15.6.0
ui RStudio
language (EN)
collate en_US.UTF-8
ctype en_US.UTF-8
tz America/New_York
date 2018-12-15

─ Packages
package * version date lib source
assertthat 0.2.0 2017-04-11 [1] CRAN (R 3.5.0)
backports 1.1.2 2017-12-13 [1] CRAN (R 3.5.0)
base64enc 0.1-3 2015-07-28 [1] CRAN (R 3.5.0)
callr 3.0.0 2018-08-24 [1] CRAN (R 3.5.0)
class 7.3-14 2015-08-30 [1] CRAN (R 3.5.1)
classInt 0.2-3 2018-04-16 [1] CRAN (R 3.5.0)
cli 1.0.1 2018-09-25 [1] CRAN (R 3.5.0)
crayon 1.3.4 2017-09-16 [1] CRAN (R 3.5.0)
DBI 1.0.0 2018-05-02 [1] CRAN (R 3.5.0)
desc 1.2.0 2018-05-01 [1] CRAN (R 3.5.0)
devtools 2.0.0 2018-10-19 [1] CRAN (R 3.5.1)
digest 0.6.18 2018-10-10 [1] CRAN (R 3.5.0)
e1071 1.7-0 2018-07-28 [1] CRAN (R 3.5.0)
fs 1.2.6 2018-08-23 [1] CRAN (R 3.5.0)
geojsonsf * 1.2.1 2018-12-03 [1] Github (e692169)
glue 1.3.0 2018-07-17 [1] CRAN (R 3.5.0)
magrittr 1.5 2014-11-22 [1] CRAN (R 3.5.0)
memoise 1.1.0 2017-04-21 [1] CRAN (R 3.5.0)
pkgbuild 1.0.2 2018-10-16 [1] CRAN (R 3.5.0)
pkgload 1.0.1 2018-10-11 [1] CRAN (R 3.5.0)
prettyunits 1.0.2 2015-07-13 [1] CRAN (R 3.5.0)
processx 3.2.0 2018-08-16 [1] CRAN (R 3.5.0)
ps 1.2.0 2018-10-16 [1] CRAN (R 3.5.0)
R6 2.3.0 2018-10-04 [1] CRAN (R 3.5.0)
Rcpp 1.0.0 2018-11-07 [1] CRAN (R 3.5.0)
remotes 2.0.1 2018-10-19 [1] CRAN (R 3.5.0)
rlang 0.3.0.1 2018-10-25 [1] CRAN (R 3.5.0)
rprojroot 1.3-2 2018-01-03 [1] CRAN (R 3.5.0)
rstudioapi 0.8 2018-10-02 [1] CRAN (R 3.5.0)
sessioninfo 1.1.0 2018-09-25 [1] CRAN (R 3.5.0)
sf * 0.7-1 2018-10-24 [1] CRAN (R 3.5.0)
spData 0.2.9.4 2018-09-15 [1] CRAN (R 3.5.1)
testthat 2.0.1 2018-10-13 [1] CRAN (R 3.5.0)
units 0.6-1 2018-09-21 [1] CRAN (R 3.5.1)
usethis 1.4.0 2018-08-14 [1] CRAN (R 3.5.0)
withr 2.1.2 2018-03-15 [1] CRAN (R 3.5.0)
yaml 2.2.0 2018-07-25 [1] CRAN (R 3.5.0)

[1] /Library/Frameworks/R.framework/Versions/3.5/Resources/library

@SymbolixAU SymbolixAU self-assigned this Dec 15, 2018
@SymbolixAU SymbolixAU added the bug label Dec 15, 2018
@SymbolixAU
Copy link
Owner

@SymbolixAU SymbolixAU commented Dec 15, 2018

thanks for reporting - this is definitely a bug. I'll take a look.

as per the specification - para 3.3

The value of "features" is a JSON array. Each element of the array is a Feature object as defined above. It is possible for this array to be empty.

So it's valid, and needs to be handled

TODO

geo <- '{"type":"FeatureCollection","features":[]}'
geojsonsf::geojson_sf( geo )
sf::st_read( geo )
  • empty features

Give section 3.2 of the geojson specification

A Feature object has a "type" member with the value "Feature".
A Feature object has a member with the name "geometry".
A Feature object has a member with the name "properties"

Then I say these cases are not permitted

'{"type":"FeatureCollection","features":[{"type":"Feature"}]}'
'{"type":"Feature"}'

and they also fail geojsonlite.com tests too

SymbolixAU added a commit that referenced this issue Dec 15, 2018
@SymbolixAU SymbolixAU added this to the v1.3 milestone Dec 16, 2018
@SymbolixAU
Copy link
Owner

@SymbolixAU SymbolixAU commented Dec 16, 2018

@sheffe I've got this working in the dev version if you want to start using & testing

devtools::install_github("SymbolixAU/geojsonsf")

And here are the tests of it working

@SymbolixAU SymbolixAU closed this Dec 17, 2018
@sheffe
Copy link
Author

@sheffe sheffe commented Dec 17, 2018

@SymbolixAU thank you very much! Looks like you had it tested and verified faster than I could revisit, but can confirm that all of the crashing cases I saw in live action are now working well.

@SymbolixAU
Copy link
Owner

@SymbolixAU SymbolixAU commented Dec 18, 2018

thanks for testing & reporting back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.