Skip to content

Sizing geom_sf point data with variable containing NAs #3483

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

Closed
daniel-barnett opened this issue Aug 13, 2019 · 4 comments · Fixed by #3491
Closed

Sizing geom_sf point data with variable containing NAs #3483

daniel-barnett opened this issue Aug 13, 2019 · 4 comments · Fixed by #3491

Comments

@daniel-barnett
Copy link
Contributor

Trying to plot point data with geom_sf() that has a mapping to a variable that contains NAs creates an error. This occurs both in the CRAN 3.2.1 and development versions.

library(ggplot2)
packageVersion("ggplot2")
#> [1] '3.2.1.9000'

# From geom_sf() examples in docs
nc <- sf::st_read(system.file("shape/nc.shp", package = "sf"), quiet = TRUE)
nc_3857 <- sf::st_transform(nc, "+init=epsg:3857")

nc_3857$mid <- sf::st_centroid(nc_3857$geometry)

ggplot(nc_3857) +
  geom_sf(aes(geometry = mid, size = AREA))

nc_3857$AREA[10] <- NA

ggplot(nc_3857) +
  geom_sf(aes(geometry = mid, size = AREA))
#> Error in checkNA("fontsize"): mixture of missing and non-missing values for fontsize

sessionInfo()
#> R version 3.6.1 (2019-07-05)
#> Platform: x86_64-w64-mingw32/x64 (64-bit)
#> Running under: Windows 7 x64 (build 7601) Service Pack 1
#> 
#> Matrix products: default
#> 
#> locale:
#> [1] LC_COLLATE=English_New Zealand.1252 
#> [2] LC_CTYPE=English_New Zealand.1252   
#> [3] LC_MONETARY=English_New Zealand.1252
#> [4] LC_NUMERIC=C                        
#> [5] LC_TIME=English_New Zealand.1252    
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] ggplot2_3.2.1.9000
#> 
#> loaded via a namespace (and not attached):
#>  [1] Rcpp_1.0.2         pillar_1.4.2       compiler_3.6.1    
#>  [4] highr_0.8          class_7.3-15       tools_3.6.1       
#>  [7] digest_0.6.20      evaluate_0.14      tibble_2.1.3      
#> [10] gtable_0.3.0       pkgconfig_2.0.2    rlang_0.4.0       
#> [13] DBI_1.0.0          yaml_2.2.0         xfun_0.8          
#> [16] e1071_1.7-2        withr_2.1.2        dplyr_0.8.3       
#> [19] stringr_1.4.0      knitr_1.23         classInt_0.4-1    
#> [22] grid_3.6.1         tidyselect_0.2.5   glue_1.3.1        
#> [25] sf_0.7-7           R6_2.4.0           rmarkdown_1.14    
#> [28] purrr_0.3.2        magrittr_1.5       units_0.6-3       
#> [31] scales_1.0.0       htmltools_0.3.6    assertthat_0.2.1  
#> [34] colorspace_1.4-1   labeling_0.3       KernSmooth_2.23-15
#> [37] stringi_1.4.3      lazyeval_0.2.2     munsell_0.5.0     
#> [40] crayon_1.3.4

Created on 2019-08-13 by the reprex package (v0.3.0)

@yutannihilation
Copy link
Member

Hmm, this change (I'm not sure if we call this a "regression") seems introduced in v3.2.0... The code used to work before:

library(ggplot2)
#> Registered S3 methods overwritten by 'ggplot2':
#>   method         from 
#>   [.quosures     rlang
#>   c.quosures     rlang
#>   print.quosures rlang
packageVersion("ggplot2")
#> [1] '3.1.1'

nc <- sf::st_read(system.file("shape/nc.shp", package = "sf"), quiet = TRUE)
nc_3857 <- sf::st_transform(nc, "+init=epsg:3857")

nc_3857$mid <- sf::st_centroid(nc_3857$geometry)

nc_3857$AREA[10] <- NA

ggplot(nc_3857) +
  geom_sf(aes(geometry = mid, size = AREA))

Created on 2019-08-13 by the reprex package (v0.3.0)

The error comes from here in sf_grob():

ggplot2/R/geom-sf.R

Lines 159 to 162 in fc8f7d5

gp <- gpar(
col = col, fill = fill, fontsize = fontsize, lwd = lwd, lty = lty,
lineend = lineend, linejoin = linejoin, linemitre = linemitre
)

Since grid::gpar() accepts a single NA, containing NAs hadn't become a problem until sf_grob() got vectorized by #3164.

grid::gpar(fontsize = NA)
#> named list()
grid::gpar(fontsize = c(1, NA))
#> Error in checkNA("fontsize"): mixture of missing and non-missing values for fontsize

Created on 2019-08-13 by the reprex package (v0.3.0)

Now that it's vectorized, it seems we need to replace NAs on ggplot2's side to let it be drawn. But, I'm not sure if NA-size points should be drawn or not. Maybe it should not, similarly to #2757 (comment)?

@yutannihilation
Copy link
Member

But, I'm not sure if NA-size points should be drawn or not.

Now I'm sure it should not. Probably, GeomSf needs to have its own handle_na() because non_missing_aes varies, depending the type of the geometry.

ggplot2/R/geom-.r

Lines 67 to 72 in b842024

handle_na = function(self, data, params) {
remove_missing(data, params$na.rm,
c(self$required_aes, self$non_missing_aes),
snake_class(self)
)
},

@yutannihilation
Copy link
Member

GeomSf needs to have its own handle_na()

Sorry, I was wrong here. It seems GeomSf should always have non_missing_aes() as sf_grob() always passes fontsize.

@lock
Copy link

lock bot commented Feb 21, 2020

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Feb 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants