R-ArcGIS / arcpbf

Rust crate and R package for processing Esri Protocol Buffers
https://r.esri.com/arcpbf/
Apache License 2.0
8 stars 0 forks source link

Regression w/ `arc_select()` returning data.frame not sf objects #1

Closed elipousson closed 4 months ago

elipousson commented 4 months ago

Describe the bug

Using the latest version of arcgislayers, arc_select() is returning a data.frame instead of an sf object. I haven't dug into the details but I suspect it is related to this PR https://github.com/R-ArcGIS/arcgislayers/pull/197/

To Reproduce

library(arcgislayers)

bikeways_url <- "https://services.arcgis.com/njFNhDsUCentVYJW/arcgis/rest/services/Bikeways_Application_gdb/FeatureServer/0"

bikeways_data <- arc_read(bikeways_url)

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

bikeways_layer <- arc_open(bikeways_url)

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

Created on 2024-07-07 with reprex v2.1.0

Expected behavior

The url should return a sf object.

Additional context

The tests should be updated to make sure this type of regression would get caught in the future.

JosiahParry commented 4 months ago

I cannot reproduce this issue:

library(arcgislayers)

furl <- "https://services.arcgis.com/njFNhDsUCentVYJW/arcgis/rest/services/Bikeways_Application_gdb/FeatureServer/0"

res <- arc_open(furl) |> 
  arc_select()

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

Created on 2024-07-08 with reprex v2.1.0

elipousson commented 4 months ago

I'm definitely still seeing this issue. I ran into it with both the CRAN version and GitHub — which I guess suggests that it is an issue on my end — but installing version 0.2.1 from this commit https://github.com/R-ArcGIS/arcgislayers/commit/c988b92760833d7593e8be7a2b3dda2a98b348ec restored the expected functionality. What version did you use when you tried to reproduce?

JosiahParry commented 4 months ago

Can you run sessionInfo()? I just reinstalled 0.3.0 from CRAN

library(arcgislayers)

furl <- "https://services.arcgis.com/njFNhDsUCentVYJW/arcgis/rest/services/Bikeways_Application_gdb/FeatureServer/0"

res <- arc_open(furl) |> 
  arc_select()

class(res)
#> [1] "sf"         "data.frame"
sessionInfo()
#> R version 4.4.0 beta (2024-04-12 r86412)
#> Platform: aarch64-apple-darwin20
#> Running under: macOS Sonoma 14.4.1
#> 
#> Matrix products: default
#> BLAS:   /Library/Frameworks/R.framework/Versions/4.4-arm64/Resources/lib/libRblas.0.dylib 
#> LAPACK: /Library/Frameworks/R.framework/Versions/4.4-arm64/Resources/lib/libRlapack.dylib;  LAPACK version 3.12.0
#> 
#> locale:
#> [1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8
#> 
#> time zone: America/New_York
#> tzcode source: internal
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] arcgislayers_0.3.0
#> 
#> loaded via a namespace (and not attached):
#>  [1] jsonify_1.2.2       compiler_4.4.0      reprex_2.1.0       
#>  [4] Rcpp_1.0.12         collapse_2.0.13     RcppSimdJson_0.1.11
#>  [7] parallel_4.4.0      yaml_2.3.8          fastmap_1.1.1      
#> [10] arcpbf_0.1.2        R6_2.5.1            curl_5.2.1         
#> [13] classInt_0.4-10     httr2_1.0.1         sf_1.0-16          
#> [16] knitr_1.46          arcgisutils_0.3.0   units_0.8-5        
#> [19] R.cache_0.16.0      DBI_1.2.2           R.utils_2.12.3     
#> [22] rlang_1.1.3         xfun_0.44           fs_1.6.3           
#> [25] cli_3.6.2           withr_3.0.0         magrittr_2.0.3     
#> [28] class_7.3-22        digest_0.6.35       grid_4.4.0         
#> [31] rappdirs_0.3.3      lifecycle_1.0.4     R.methodsS3_1.8.2  
#> [34] R.oo_1.26.0         vctrs_0.6.5         KernSmooth_2.23-22 
#> [37] proxy_0.4-27        evaluate_0.23       glue_1.7.0         
#> [40] styler_1.10.3       e1071_1.7-14        rmarkdown_2.26     
#> [43] purrr_1.0.2         tools_4.4.0         htmltools_0.5.8.1

Created on 2024-07-08 with reprex v2.1.0

elipousson commented 4 months ago
library(arcgislayers)

bikeways_url <- "https://services.arcgis.com/njFNhDsUCentVYJW/arcgis/rest/services/Bikeways_Application_gdb/FeatureServer/0"

bikeways_data <- arc_read(bikeways_url)

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

sessionInfo()
#> R version 4.4.0 (2024-04-24)
#> Platform: aarch64-apple-darwin20
#> Running under: macOS Sonoma 14.5
#> 
#> Matrix products: default
#> BLAS:   /Library/Frameworks/R.framework/Versions/4.4-arm64/Resources/lib/libRblas.0.dylib 
#> LAPACK: /Library/Frameworks/R.framework/Versions/4.4-arm64/Resources/lib/libRlapack.dylib;  LAPACK version 3.12.0
#> 
#> locale:
#> [1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8
#> 
#> time zone: America/New_York
#> tzcode source: internal
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] arcgislayers_0.3.0
#> 
#> loaded via a namespace (and not attached):
#>  [1] jsonify_1.2.2       compiler_4.4.0      reprex_2.1.0       
#>  [4] Rcpp_1.0.12         RcppSimdJson_0.1.11 yaml_2.3.8         
#>  [7] fastmap_1.2.0       arcpbf_0.1.2        R6_2.5.1           
#> [10] curl_5.2.1          classInt_0.4-10     httr2_1.0.1        
#> [13] sf_1.0-16           knitr_1.46          arcgisutils_0.3.0  
#> [16] units_0.8-5         R.cache_0.16.0      DBI_1.2.2          
#> [19] R.utils_2.12.3      rlang_1.1.4         xfun_0.44          
#> [22] fs_1.6.4            cli_3.6.3.9000      withr_3.0.0        
#> [25] magrittr_2.0.3      class_7.3-22        digest_0.6.35      
#> [28] grid_4.4.0          rstudioapi_0.16.0   rappdirs_0.3.3     
#> [31] lifecycle_1.0.4     R.methodsS3_1.8.2   R.oo_1.26.0        
#> [34] vctrs_0.6.5         KernSmooth_2.23-24  data.table_1.15.4  
#> [37] proxy_0.4-27        evaluate_0.23       glue_1.7.0         
#> [40] styler_1.10.3       e1071_1.7-14        rmarkdown_2.27     
#> [43] purrr_1.0.2         tools_4.4.0         htmltools_0.5.8.1

Created on 2024-07-08 with reprex v2.1.0

JosiahParry commented 4 months ago

Okay, I can repro but its not related to arcpbf or anything. It is due to the fact that you don't have collapse installed but also have data.table installed. I figured this out by removing collapse then i removed data.table. When collapse is present, there is no problem. When it isn't but data.table is, there's a problem. When both collapse and data.table aren't present its not a problem.

So it is this bit of arcgisutils:::rbind_results()

    else if (rlang::is_installed("data.table")) {
        x <- data.table::rbindlist(x)
        data.table::setDF(x)
    }

Edit: it is because of arcpbf:::squish_dfs() which should be replaced with arcgisutils:::rbind_results()

JosiahParry commented 4 months ago

@elipousson in about an hour can you update {arcpbf} from https://r-arcgis.r-universe.dev/arcpbf? If that works for you ill push an update to CRAN.

elipousson commented 4 months ago

@JosiahParry Tried it and got it working! I can double-check later if I needed to wait an hour exactly. Always good news when a bug report is a real bug and not just user error. Especially when the bug is a relatively quick fix.

JosiahParry commented 4 months ago

Awesome! The one hour this is because r-universe rebuilds binaries on an hour schedule if there has been a change

elipousson commented 4 months ago

Did you want me to add a test with expect_s3_class to check for an sf output? This is probably a one and done fix but would be good to get the expected class output into the text coverage.

JosiahParry commented 4 months ago

Sure! That’s not a bad idea! The challenge is being able to test having different packages available

JosiahParry commented 4 months ago

@elipousson new version of arcpbf hit CRAN this morning. Please let me know if it all goes well