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

purrr::list_cbind() and vctrs::vec_cbind() returning a data.frame instead of a sf object #2157

Open
elipousson opened this issue Apr 29, 2023 · 5 comments

Comments

@elipousson
Copy link

Combining a simple feature object with a data.frame using purrr::list_cbind() or vctrs::vec_cbind() returns a data.frame instead of an sf object as expected.

nc <- sf::st_read(system.file("shape/nc.shp", package = "sf"))
#> Reading layer `nc' from data source 
#>   `/Users/elipousson/Library/R/arm64/4.2/library/sf/shape/nc.shp' 
#>   using driver `ESRI Shapefile'
#> Simple feature collection with 100 features and 14 fields
#> Geometry type: MULTIPOLYGON
#> Dimension:     XY
#> Bounding box:  xmin: -84.32385 ymin: 33.88199 xmax: -75.45698 ymax: 36.58965
#> Geodetic CRS:  NAD27

new_col <- data.frame("new_col" = seq_len(length(nc$NAME)))

nc_bind_cols <- dplyr::bind_cols(
  nc,
  new_col
)

class(nc_bind_cols)
#> [1] "sf"         "data.frame"

nc_list_cbind <- purrr::list_cbind(
  list(
    nc,
    new_col
  )
)

class(nc_list_cbind)
#> [1] "data.frame"

nc_vec_cbind <- vctrs::vec_cbind(
  nc,
  new_col
)

class(nc_vec_cbind)
#> [1] "data.frame"

Created on 2023-04-29 with reprex v2.0.2

I'm pretty sure that both approaches worked with sf objects previously so I'm unsure if there was a change in vctrs or a change in sf.

R version 4.2.3 (2023-03-15)
Platform: aarch64-apple-darwin20 (64-bit)
Running under: macOS Big Sur 11.7.1

Matrix products: default
LAPACK: /Library/Frameworks/R.framework/Versions/4.2-arm64/Resources/lib/libRlapack.dylib

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] sinew_0.4.0        pak_0.4.0          devtools_2.4.5     usethis_2.1.6.9000

loaded via a namespace (and not attached):
 [1] tidyselect_1.2.0   remotes_2.4.2      rematch2_2.1.2     purrr_1.0.1       
 [5] sf_1.0-12          generics_0.1.3     vctrs_0.6.2        miniUI_0.1.1.1    
 [9] htmltools_0.5.5    yaml_2.3.7         utf8_1.2.3         rlang_1.1.0.9000  
[13] pkgbuild_1.4.0     e1071_1.7-13       urlchecker_1.0.1   later_1.3.0       
[17] pillar_1.9.0       glue_1.6.2         DBI_1.1.3          sessioninfo_1.2.2 
[21] lifecycle_1.0.3    stringr_1.5.0      htmlwidgets_1.6.2  memoise_2.0.1     
[25] callr_3.7.3        fastmap_1.1.1      httpuv_1.6.9       ps_1.7.4          
[29] class_7.3-21       fansi_1.0.4        Rcpp_1.0.10        KernSmooth_2.23-20
[33] xtable_1.8-4       promises_1.2.0.1   classInt_0.4-9     cachem_1.0.7      
[37] pkgload_1.3.2      mime_0.12          fs_1.6.1           sos_2.1-4         
[41] brew_1.0-8         digest_0.6.31      stringi_1.7.12     processx_3.8.0    
[45] dplyr_1.1.1        shiny_1.7.4        grid_4.2.3         cli_3.6.1         
[49] tools_4.2.3        magrittr_2.0.3     proxy_0.4-27       tibble_3.2.1      
[53] profvis_0.3.7      crayon_1.5.2       pkgconfig_2.0.3    ellipsis_0.3.2    
[57] prettyunits_1.1.1  rstudioapi_0.14    R6_2.5.1           units_0.8-1       
[61] compiler_4.2.3    
 GEOS           GDAL         proj.4 GDAL_with_GEOS     USE_PROJ_H           PROJ 
 "3.11.0"        "3.5.3"        "9.1.0"         "true"         "true"        "9.1.0" 
@francisbarton
Copy link

francisbarton commented Jun 26, 2023

Hi @elipousson I have just seen this. I already raised a similar issue (though relating to list_rbind()) on the purrr repo. (Though a few days after you raised this issue!)
tidyverse/purrr#1076

@elipousson
Copy link
Author

It wouldn't be a big issue but purrr::list_rbind() is being recommended as the replacement for the deprecated purrr::map_dfr() (and the row-binding functions have the same issue as the column-binding function) so I expect there may be some confused users. It is relatively straightforward to fix in a script but I'm not sure how it could be handled by sf or if vctrs or purrr would allow this edge case.

Here is an example showind the issue with purrr::list_rbind() (and the simple fix):

nc <- sf::read_sf(system.file("shape/nc.shp", package = "sf"))

nc_bind_rows <- dplyr::bind_rows(
  nc[2, ],
  nc[1, ]
)

class(nc_bind_rows)
#> [1] "sf"         "tbl_df"     "tbl"        "data.frame"

nc_vec_rbind <- vctrs::vec_rbind(
  nc[2, ],
  nc[1, ]
)

class(nc_vec_rbind)
#> [1] "data.frame"

class(sf::st_as_sf(nc_vec_rbind))
#> [1] "sf"         "data.frame"

Created on 2023-06-26 with reprex v2.0.2

@francisbarton
Copy link

I do think this is an issue with vctrs or purrr not with sf though.

@hadley
Copy link
Contributor

hadley commented Jul 26, 2023

To be clear, map_dfr() is not deprecated; it's superseded.

@elipousson
Copy link
Author

Thanks for that reminder!

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

No branches or pull requests

3 participants