Allow to convert to other types, including auto

[ateucher]

Not ready for merge.

Right now we currently only support FeatureCollection and GeometryCollection, but there's no reason we couldn't expand that. I started on this by modifying the geoclass function to accept all types, including 'auto' using the new geojson::to_geojson function.

This looks like it works well for sf and Spatial objects, but to implement for other classes will need more work to accommodate the different types. num_to_geo_list and list_to_geo_list will need to be modified at the very least. Or we could (temporarily?) restrict the geojson_json methods for list, data.frame, and numeric to deal only support FeatureCollection and GeometryCollection as they do now, and kick this one down the road...

[codecov-io]

Codecov Report

Merging #125 into master will increase coverage by 0.17%. The diff coverage is 50%.

@@            Coverage Diff             @@
##           master     #125      +/-   ##
==========================================
+ Coverage   63.64%   63.81%   +0.17%     
==========================================
  Files          29       29              
  Lines        1433     1440       +7     
==========================================
+ Hits          912      919       +7     
  Misses        521      521

Impacted Files	Coverage Δ
R/zzz.r	59.14% <100%> (+1.14%)	:arrow_up:
R/geojson_json.R	51.42% <7.69%> (ø)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 43a5bb8...5e762b0. Read the comment docs.

[ateucher]

Ok, I lied above. sp objects are always forced to FeatureCollections because that's what rgdal::writeOGR writes them as (in geojson_rw()) (as documented already, I just didn't notice).

So currently, for sf objects, the type argument works. Previous to using geoclass() and passing the type argument on to that, they were automatically converted to the appropriate type, now they are converted to FeatureCollection by default (As that is the default value of type in geojson_json - it was just ignored previously).

To return to the previous behaviour for sf objects, we could set the default value of type to 'auto' in the sf/c/g methods, which I think we should do... (would require reverting 9dbe56976d49).

[sckott]

thanks will have a look, sorry for delay was on away vacay on weekend

to be clear, do you think it's good to go now, assuming i'm on board with changes?

[ateucher]

Definitely no need to apologize - it was the weekend!

I think something needs to be done with data.frame, list, and numeric methods - currently only type = 'GeometryCollection' and type = 'FeatureCollection' will work, so if we want to get this out sooner than later I think we could just restrict the type argument to those, so if someone calls geojson_json(x, type = 'auto'), where x is a data.frame, list, or numeric, it doesn't get a strange error deep down in the call stack.

[sckott]

thanks for clarification

[sckott]

@ateucher hmm, for sp classes, looks like you have a pattern like (beware, pseudocode)

geojson_json.SpatialPolygons <- function(other_params, type='FeatureCollection') {
   geoclass(other_params, type = 'FeatureCollection')
}

so type param is hard-coded. Not sure if that was on purpose or not. I could see how it would be on purpose if we want to restrict only to FeatureCollection

---

For data.frame/numeric/list, does this look good?

# define a helper function
check_type <- function(x) {
    types <- c('FeatureCollection', 'GeometryCollection')
  if (!x %in% types) stop("'type' must be one of: ", paste0(types, collapse=", "))
}

# use at the top of each function data.frame/numeric/list methods
check_type(type) 

geojson_json(c(-99.74,32.45), type="foobar")
#> Error in check_type(type) :
#>   'type' must be one of: FeatureCollection, GeometryCollection

[ateucher]

@sckott that's just what I was thinking as well. For sp classes, yes the hard-coded type = 'FeatureCollection' was intentional. I'm pretty sure rgdal::writeOGR will always write a FeatureCollection regardless of whether or not the sp object has attributes.

[ateucher]

If you have the time, I'm happy if you merge this then add the check_type for those classes. Then I think we'll be good to go

[sckott]

Okay, will do the merge then add the type checker for those methods.

submitting geojson 2 cran now, so can do this pkg submission tomorrow -ish

[ateucher]

Awesome, thanks so much @sckott

[github-actions[bot]]

This pull request 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.