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

default_aesthetics in geom_sf does not handle NULL properly #3963

Closed
wesnm opened this issue Apr 26, 2020 · 9 comments · Fixed by #3964
Closed

default_aesthetics in geom_sf does not handle NULL properly #3963

wesnm opened this issue Apr 26, 2020 · 9 comments · Fixed by #3964

Comments

@wesnm
Copy link

wesnm commented Apr 26, 2020

When plotting a map using a discrete fill scale, I get the error below. The source seems to be default_aesthetics not handling a NULL value for "type" (the line numbers may be off, I was adding some debugging prints to check out internal values):

ggplot(nyt$state.maps[!(STATE %in% c("Alaska", "Hawaii", "Puerto Rico"))]) +
    geom_sf(aes(geometry=GEOMETRY, fill=INFECTIONS))

 Error in if (type == "point") { : argument is of length zero 
18. default_aesthetics(params$legend) at geom-sf.R#113
17. modify_list(default_aesthetics(params$legend), data) at geom-sf.R#113
16. f(...) at ggproto.r#178
15. g$draw_key(g$data[i, ], g$params, key_size) at guide-legend.r#635
14. FUN(X[[i]], ...) 
13. lapply(guide$geoms, function(g) {
    g$draw_key(g$data[i, ], g$params, key_size)
}) at guide-legend.r#634
12. FUN(X[[i]], ...) 
11. lapply(seq_len(nbreak), draw_key) at guide-legend.r#639
10. unlist(lapply(seq_len(nbreak), draw_key), recursive = FALSE) at guide-legend.r#639
9. guide_gengrob.legend(X[[i]], ...) at guides-.r#356
8. FUN(X[[i]], ...) 
7. lapply(gdefs, guide_gengrob, theme) at guides-.r#249
6. guides_gengrob(gdefs, theme) at guides-.r#137
5. build_guides(plot$scales, plot$layers, plot$mapping, position, 
    theme, plot$guides, plot$labels) at plot-build.r#179
4. ggplot_gtable.ggplot_built(data) at plot-build.r#158
3. ggplot_gtable(data) at plot.r#173
2. print.ggplot(x) 
1. (function (x, ...) 
UseMethod("print"))(x) 

Passing a value for legend (or character value for show.legend) works fine:

ggplot(nyt$state.maps[!(STATE %in% c("Alaska", "Hawaii", "Puerto Rico"))]) +
    geom_sf(aes(geometry=GEOMETRY, fill=INFECTIONS_CUT), legend="meh")

You can repeat this using the data from this stack overflow post this stack overflow post and some small changes. Moving the fill aesthetic inside the aes mapping gives the error:

Error:
ggplot2::ggplot() + 
    geom_sf(data = gainsville_df, aes(geometry=Geomtry, fill=`Cluster Group`), alpha=0.2) +
    coord_sf(crs = "+init=epsg:4326")+
    scale_fill_manual(values = c("red", "grey", "seagreen3","gold", "green","orange"), name= "Cluster Group")

No Error:
ggplot2::ggplot() + 
    geom_sf(data = gainsville_df, aes(geometry=Geomtry, fill=`Cluster Group`), alpha=0.2, legend="meh") +
    coord_sf(crs = "+init=epsg:4326")+
    scale_fill_manual(values = c("red", "grey", "seagreen3","gold", "green","orange"), name= "Cluster Group")
@yutannihilation

This comment has been minimized.

@yutannihilation

This comment has been minimized.

@yutannihilation
Copy link
Member

yutannihilation commented Apr 26, 2020

Confirmed. Here's a minimal reprex. The error happens when the data is NOT an sf and the scale of colour or fill is discrete.

library(ggplot2)

nc <- sf::read_sf(system.file("shape/nc.shp", package="sf"))
nc$BIR74_bin <- cut_number(nc$BIR74, 3)
nc_tibble <- tibble::as_tibble(nc)

ggplot(nc) +
  geom_sf(aes(fill = BIR74_bin))

ggplot(nc_tibble) +
  geom_sf(aes(geometry = geometry, fill = BIR74_bin))
#> Error in if (type == "point") {: argument is of length zero

Created on 2020-04-26 by the reprex package (v0.3.0)

@clauswilke
Copy link
Member

clauswilke commented Apr 26, 2020

Specifically, things work the moment show.legend is specified (see below). This suggests the problem is in the legend detection code. I suspect this if branch needs an else:

ggplot2/R/layer-sf.R

Lines 51 to 59 in d3d47be

if (is_sf(data)) {
sf_type <- detect_sf_type(data)
if (sf_type == "point") {
self$geom_params$legend <- "point"
} else if (sf_type == "line") {
self$geom_params$legend <- "line"
} else {
self$geom_params$legend <- "polygon"
}

library(ggplot2)

nc <- sf::read_sf(system.file("shape/nc.shp", package="sf"))
nc$BIR74_bin <- cut_number(nc$BIR74, 3)
nc_tibble <- tibble::as_tibble(nc)

# doesn't work
ggplot(nc_tibble) +
  geom_sf(aes(geometry = geometry, fill = BIR74_bin))
#> Error in if (type == "point") {: argument is of length zero

# works
ggplot(nc_tibble) +
  geom_sf(aes(geometry = geometry, fill = BIR74_bin), show.legend = "polygon")

Created on 2020-04-26 by the reprex package (v0.3.0)

@clauswilke
Copy link
Member

clauswilke commented Apr 26, 2020

I've created a PR to fix this: #3964

@yutannihilation
Copy link
Member

yutannihilation commented Apr 26, 2020

Related problem. Maybe p2 should show a line legend?

library(ggplot2)
library(patchwork)

p1 <- rbind(c(1,1), c(2,2), c(3,3))
s1 <- rbind(c(0,3), c(0,4), c(1,5), c(2,5))
s2 <- rbind(c(0.2,3), c(0.2,4), c(1,4.8), c(2,4.8))
s3 <- rbind(c(0,4.4), c(0.6,5))

d_sf <- sf::st_sf(
  g_point = sf::st_sfc(sf::st_multipoint(p1)),
  g_line = sf::st_sfc(sf::st_multilinestring(list(s1,s2,s3))),
  v = "a"
)

p1 <- ggplot(d_sf) +
  geom_sf(aes(geometry = g_point, colour = "a"))

p2 <- ggplot(d_sf) +
  geom_sf(aes(geometry = g_line, colour = "a"))

p1 / p2

Created on 2020-04-26 by the reprex package (v0.3.0)

@yutannihilation
Copy link
Member

yutannihilation commented Apr 26, 2020

I mean, the absence of else branch is a problem, but it seems we should use geometry aesthetic to determine detect_sf_type() in general.

@clauswilke
Copy link
Member

clauswilke commented Apr 26, 2020

Good point. I'll look into it tomorrow.

@clauswilke
Copy link
Member

clauswilke commented Apr 26, 2020

It's getting a bit convoluted, but here you go.

library(ggplot2)
library(patchwork)

p1 <- rbind(c(1,1), c(2,2), c(3,3))
s1 <- rbind(c(0,3), c(0,4), c(1,5), c(2,5))
s2 <- rbind(c(0.2,3), c(0.2,4), c(1,4.8), c(2,4.8))
s3 <- rbind(c(0,4.4), c(0.6,5))

d_sf <- sf::st_sf(
  g_point = sf::st_sfc(sf::st_multipoint(p1)),
  g_line = sf::st_sfc(sf::st_multilinestring(list(s1,s2,s3))),
  v = "a"
)

p1 <- ggplot(d_sf) +
  geom_sf(aes(geometry = g_point, colour = "a"))

p2 <- ggplot(d_sf) +
  geom_sf(aes(geometry = g_line, colour = "a"))

p1 / p2

Created on 2020-04-26 by the reprex package (v0.3.0)

clauswilke added a commit that referenced this issue Apr 27, 2020
* make legend detection code more robust. fixes #3963

* properly infer geometry from the right column

* add unit test

* add news.md
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

Successfully merging a pull request may close this issue.

3 participants