Better respect theme settings when drawing coord_sf() panel grid

[clauswilke]

This PR makes coord_sf() behave more similarly to coord_cartesian() when drawing the panel grid, by applying the same line-width correction and by respecting element_blank(). Closes #2991 and #2525.

Reprex 1:

library(ggplot2)

df <- data.frame(x = c(1, 2, 3), y = c(1, 2, 3))
p <- ggplot(df, aes(x, y)) + geom_point()

# default grid
cowplot::plot_grid(
  p + coord_fixed() + theme(panel.grid.minor = element_blank()) +
    ggtitle("coord_fixed()"),
  p + coord_sf() +
    ggtitle("coord_sf()")
)

# different theme
cowplot::plot_grid(
  p + coord_fixed() + theme_bw() + theme(panel.grid.minor = element_blank()) +
    ggtitle("coord_fixed()"),
  p + coord_sf() + theme_bw() + 
    ggtitle("coord_sf()")
)

# setting grid lines to element_blank()
cowplot::plot_grid(
  p + coord_fixed() + theme(panel.grid = element_blank()) +
    ggtitle("coord_fixed()"),
  p + coord_sf() + theme(panel.grid = element_blank()) +
    ggtitle("coord_sf()")
)

^{Created on 2019-02-01 by the reprex package (v0.2.1)}

Reprex 2:

library(sf) 
#> Linking to GEOS 3.6.1, GDAL 2.1.3, PROJ 4.9.3
library(ggplot2)

nc <- st_read(system.file("gpkg/nc.gpkg", package = "sf"), quiet = TRUE) 

ggplot(nc) + 
  geom_sf(data = nc, aes(fill = AREA)) + 
  theme_void()

^{Created on 2019-02-01 by the reprex package (v0.2.1)}

[clauswilke]

@lionel- Do you think it's possible to add some visual tests for coord_sf() if we don't actually use geom_sf() (just like the first reprex here)? Should that work, or would you also expect it to be unstable and depend on the specific versions of GDAL, PROJ, etc.?

[clauswilke]

@thomasp85 You previously commented:

I think this makes sense to change irrespectively of the changes in the wild it may affect... but better wait until the next big(ish) release

Do you feel we're at that point of the development cycle now? It seems to me we're making substantial changes throughout the codebase.

[thomasp85]

Pretty sure next release will be major as I’m working on two substantial PRs (one of them sf related so we might as well combine that in the same release)

[clauswilke]

To answer my own question about visual tests: I think whenever a graticule is drawn the result may depend on the specific version of sf at least.

[thomasp85]

It will very much depend on sf

[clauswilke]

Well, I've added one visual test, for the case where no graticule is drawn and no CRS is set (and we only use geom_point()). I think it should be fine, but let's see whether it passes on Travis.

[clauswilke]

I just noticed that this PR currently introduces a regression (see below). I'll need to fix this before it can be merged.

library(ggplot2)

df <- data.frame(x = c(1, 2, 3), y = c(1, 2, 3))
p <- ggplot(df, aes(x, y)) + 
  geom_point() +
  theme_void() +
  theme(panel.grid.major = element_line(color = "white"))

# works fine with coord_cartesian()
p

# doesn't work
p + coord_sf()
#> Error in check.length(gparname): 'gpar' element 'lwd' must not be length 0

^{Created on 2019-02-03 by the reprex package (v0.2.1)}

[lionel-]

@lionel- Do you think it's possible to add some visual tests for coord_sf() if we don't actually use geom_sf() (just like the first reprex here)? Should that work, or would you also expect it to be unstable and depend on the specific versions of GDAL, PROJ, etc.?

Sorry I am not familiar with sf.

[clauswilke]

@lionel- No problem. I think there's a broader issue here with geoms that may produce visually identical output via different but equivalent mathematical representations. I'm running into this with the isoband library right now. I'll open an issue on vdiffr.

[lionel-]

The solution is probably to use bitmap diffing with a threshold for these cases.

[clauswilke]

@thomasp85 Any concerns about the specific visual test I added? I've reproduced it here for your reference. Because it doesn't have a panel grid, I think it'll be robust to changes in sf, but I'll remove it if you think it remains a risk.

library(ggplot2)

df <- data.frame(x = c(1, 2, 3), y = c(1, 2, 3))
ggplot(df, aes(x, y)) +
  geom_point() +
  coord_sf() +
  theme_gray() + # to test for presence of background grob
  theme(panel.grid = element_blank())

^{Created on 2019-02-04 by the reprex package (v0.2.1)}

[clauswilke]

Unless anybody has any other concerns I'm going to go ahead and merge this. In the unlikely event that the sf visual test starts acting up in the future we can always disable it then.

[lock]

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/