leafpop icon indicating copy to clipboard operation
leafpop copied to clipboard

popupTable should drop geometry column by default for sf objects

Open davidhodge931 opened this issue 3 years ago • 4 comments

Currently it only specifies the geometry type for sf objects, which is not that useful or consistent with how it works for sp objects.

library(leaflet)
library(leafpop)

leaflet() %>%
  addTiles() %>%
  addCircleMarkers(data = breweries91,
                   popup = popupTable(breweries91))

breweries91 <- sf::st_as_sf(breweries91)

leaflet() %>%
  addTiles() %>%
  addCircleMarkers(data = breweries91,
                   popup = popupTable(breweries91))

A way to modify this might be that if it is an sf object, popupTable should relocate geometry to the last col and then default the zcol to be from cols 1 to the second to last col.

  popup_data <- data %>%
      dplyr::relocate(.data$geometry, .after = tidyselect::last_col()) 
  
  popup <- leafpop::popupTable(popup_data, zcol = 1:ncol(popup_data) - 1)

davidhodge931 avatar Sep 27 '21 01:09 davidhodge931

How bout a drop_geomerty argument (default FALSE to keep the backward compatibility)?

tim-salabim avatar Sep 27 '21 06:09 tim-salabim

That sounds good, but it still might not be backward compatible for when people supply an SP object

davidhodge931 avatar Sep 27 '21 09:09 davidhodge931

Yeah, but at least they can get the same output by setting it ti TRUE. I meant backward more with respect to the current implementation for sf objects. sp will be retired soon anyway, so users will need to switch to sf in any case

tim-salabim avatar Sep 27 '21 09:09 tim-salabim

Cool, that'd be fantastic to have that argument @tim-salabim

davidhodge931 avatar Sep 27 '21 23:09 davidhodge931