r-spatial / mapview

Interactive viewing of spatial data in R
https://r-spatial.github.io/mapview/
GNU General Public License v3.0
519 stars 90 forks source link

Error when using `popup = leafpop::popupTable(...)` with OSM data #457

Closed BastienFR closed 1 year ago

BastienFR commented 1 year ago

I've encounter a weird bug when trying to use leafpop::popupTable(...) with my data from Open Street Map. It a problem that seems really data specific, but at the same time I can't pin point what is wrong and it seems like a bug to me so I'm posting it here.

library(mapview)
library(sf)
library(osmdata)

lieux <- opq(bbox=st_bbox(c(xmin = -73.8, ymin = 45.4, xmax = -73.5, ymax = 45.5))) |>
  add_osm_feature(key="amenity", value="place_of_worship") |>
  osmdata_sf()

sh <- lieux$osm_polygons[, c("osm_id", "name", "religion")]

mapview(sh)  # Works
mapview(sh, leafpop::popupTable(sh, zcol = c("name", "religion")))  # give an error

Gives the error:

Error in map$x : $ operator is invalid for atomic vectors

When I say it's data specific, the same use of popupTable works on these shapes:

brew <- st_as_sf(leaflet::breweries91)
mapview(brew, popup = leafpop::popupTable(brew, zcol = c("brewery", "village", "founded")))  # works

nc = st_read(system.file("shape/nc.shp", package="sf"))
mapview(nc, popup = leafpop::popupTable(nc, zcol = c("AREA", "NAME")))  # works

I've played with the data, remove rows, column, etc, and I always have the same error.

I lived the problem on 2 different machines. One of them:

sessionInfo()
R version 4.3.1 (2023-06-16 ucrt)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19045)

Matrix products: default

locale:
[1] LC_COLLATE=English_Canada.1252  LC_CTYPE=English_Canada.1252   
[3] LC_MONETARY=English_Canada.1252 LC_NUMERIC=C                   
[5] LC_TIME=English_Canada.1252    

time zone: America/Toronto
tzcode source: internal

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

other attached packages:
[1] osmdata_0.2.3  sf_1.0-14      mapview_2.11.0

loaded via a namespace (and not attached):
 [1] rappdirs_0.3.3          utf8_1.2.3              generics_0.1.3         
 [4] xml2_1.3.5              class_7.3-22            KernSmooth_2.23-21     
 [7] lattice_0.21-8          digest_0.6.33           magrittr_2.0.3         
[10] RColorBrewer_1.1-3      timechange_0.2.0        grid_4.3.1             
[13] fastmap_1.1.1           jsonlite_1.8.7          e1071_1.7-13           
[16] DBI_1.1.3               fansi_1.0.4             crosstalk_1.2.0        
[19] scales_1.2.1            brew_1.0-8              codetools_0.2-19       
[22] httr2_0.2.3             cli_3.6.1               rlang_1.1.1            
[25] leafpop_0.1.0           units_0.8-2             ellipsis_0.3.2         
[28] munsell_0.5.0           yaml_2.3.7              base64enc_0.1-3        
[31] tools_4.3.1             raster_3.6-23           uuid_1.1-0             
[34] dplyr_1.1.2             colorspace_2.1-0        webshot_0.5.5          
[37] curl_5.0.1              vctrs_0.6.3             R6_2.5.1               
[40] png_0.1-8               lubridate_1.9.2         stats4_4.3.1           
[43] proxy_0.4-27            lifecycle_1.0.3         classInt_0.4-9         
[46] leaflet_2.1.2           leaflet.providers_1.9.0 htmlwidgets_1.6.2      
[49] pkgconfig_2.0.3         terra_1.7-39            pillar_1.9.0           
[52] glue_1.6.2              Rcpp_1.0.11             systemfonts_1.0.4      
[55] tibble_3.2.1            tidyselect_1.2.0        htmltools_0.5.5        
[58] svglite_2.1.1           leafem_0.2.0            satellite_1.0.4        
[61] compiler_4.3.1          sp_2.0-0       
ag5tc commented 1 year ago

I think I see the root of the issue. Your line of code that gives the error didn't include the popup = parameter for the popupTable, so mapview is trying to pass it to the map parameter.

Your code was: mapview(sh, leafpop::popupTable(sh, zcol = c("name", "religion"))) # give an error

Should be: mapview(sh, popup = leafpop::popupTable(sh, zcol = c("name", "religion"))) # this works!

BastienFR commented 1 year ago

OMG... I feel like an idiot. That's is such a rookie mistake, I can't believe I didn't catch it... Hopefully this post will disappear deep down the internet far from everybody's sight!

tim-salabim commented 1 year ago

No worries, we all make mistakes! I didn't catch it either... Was going to investigate over the weekend 😉