dcooley / sfheaders

Build sf objects from R and Rcpp
https://dcooley.github.io/sfheaders/
Other
74 stars 5 forks source link

generic list column (when sfc class is lost) #71

Closed mdsumner closed 4 years ago

mdsumner commented 4 years ago

When subsetting a sf_df without sf the sfc class and subclass is lost:

## sf is not loaded
x <- silicate::minimal_mesh
## problem
sfheaders::sf_to_df(x[1, ])
# Error in rcpp_sf_to_df(sf, fill) : 
#   Not compatible with STRSXP: [type=NULL]. 

## because 
class(x[1, ][[attr(x, "sf_column")]])
# [1] "list"

Load sf and no problem:

#compare
library(sf)
class(x[1, ][[attr(x, "sf_column")]])
sfheaders::sf_to_df(x[1, ])

I see most of this occurs in C++, so I'm not sure if my take is right, and likely I'd be unable to PR it - so FYI.

In silicate I have sc_coord methods for all and sundry, but for this situation I include sc_coord.list, which catches the "list of sfg geometries" with a simple test before the last gasp in sc_coord.default:

https://github.com/hypertidy/silicate/blob/master/R/00_coord.R#L45-L57

That subtlety has been biting me on and off for a while now, so the clarity was refreshing, but presumably it's a bit more complicated for geometrycollections.

(On reflection I realize silicate could use sfheaders lib for these decompositions, too).

SymbolixAU commented 4 years ago

When subsetting a sf_df without sf the sfc class and subclass is lost

Which begs the question, is it still a valid sfc object?

I've gone with the answer 'no'.

But it might be useful to have a better error message to say "have you loaded library(sf)" (like how I handle it in mapdeck, similar to you).

mdsumner commented 4 years ago

Please humour me, and please consider making this a warning and not stopping - :) - what benefit comes from throwing an error here?

(If it's complicated to achieve because of how things are arranged I will do the work)

SymbolixAU commented 4 years ago

I'll see what I can do.

In the C++ code it expects valid sf, sfc and sfg objects, and so looks for the required attributes. If it can't determine what type the sfc and sfg objects are from the attributes, then it won't know how to work with them.

But, it looks like even though the sfc attributes are gone, the sfg one remains. So there might be some scope to get this working.

x[1 , ]$geom[[1]]
[[1]]
[[1]][[1]]
        x   y
[1,] 0.00 0.0
[2,] 0.00 1.0
[3,] 0.75 1.0
[4,] 1.00 0.8
[5,] 0.50 0.7
[6,] 0.80 0.6
[7,] 0.69 0.0
[8,] 0.00 0.0

[[1]][[2]]
       x   y
[1,] 0.2 0.2
[2,] 0.5 0.2
[3,] 0.5 0.4
[4,] 0.3 0.6
[5,] 0.2 0.4
[6,] 0.2 0.2

attr(,"class")
[1] "XY"           "MULTIPOLYGON" "sfg"  
SymbolixAU commented 4 years ago

On branch issue71 - try it now (seemingly easier than I first imagined, and actually getting POINTs quicker was causing this break).

remotes::install_github("dcooley/sfheaders", ref = "issue71")
x <- silicate::minimal_mesh
head( sfheaders::sf_to_df(x[1, ]) )

#   sfg_id multipolygon_id polygon_id linestring_id    x   y
# 1      1               1          1             1 0.00 0.0
# 2      1               1          1             1 0.00 1.0
# 3      1               1          1             1 0.75 1.0
# 4      1               1          1             1 1.00 0.8
# 5      1               1          1             1 0.50 0.7
# 6      1               1          1             1 0.80 0.6
mdsumner commented 4 years ago

Thanks! Oh btw, I recently added "list" to the class of minimal_mesh$geom as discussed in a tibble context - so it's perhaps not the best example data set to use (sorry about that).

SymbolixAU commented 4 years ago

I think this is working now. Let me know if it causes you more grief.

mdsumner commented 4 years ago

awesome, thanks so much