Refactor geom_contour()?

[clauswilke]

As discussed on Twitter here and previously explained in a blog post by @thomasp85 an argument can be made that geom_contour() should take a 2d density map as input and calculate its own contour lines without using a stat. I'm opening this issue so we can discuss pros and cons of various approaches.

I think there is precedent for this kind of refactoring with the refactoring of geom_bar(), geom_histogram(), and geom_col() some time back.

[clauswilke]

One of my personal pet peeves is that stat_density_2d() returns a contour by default. I've always thought that is confusing.

library(ggplot2)

set.seed(4393)
dsmall <- diamonds[sample(nrow(diamonds), 1000), ]
d <- ggplot(dsmall, aes(x, y))

# works
d + geom_raster(aes(fill = stat(density)), stat = "density_2d", contour = FALSE)

# doesn't work but should in my opinion
d + geom_raster(aes(fill = stat(density)), stat = "density_2d")
#> Don't know how to automatically pick scale for object of type function. Defaulting to continuous.
#> Error in is.finite(x): default method not implemented for type 'closure'

^{Created on 2018-12-21 by the reprex package (v0.2.1)}

[clauswilke]

The argument for changing the default of the contour argument to FALSE in stat_density_2d() if we change the input data for geom_contour() is that then stat_density_2d() will work by default with the new geom_contour() and also with geom_tile() and geom_raster(). By contrast, if we leave contour = TRUE it won't work with either.

The argument against changing this default is that it will potentially break existing code.

One way around this problem may be to soft-deprecate stat_density_2d() and introduce a new stat_2d_density(). This will give users of the current stat_density_2d() time to adjust their code while introducing a new function that can be designed cleanly and without baggage.

[clauswilke]

One other issue: geom_contour() doesn't currently understand the fill aesthetic. I see no principled reason why it shouldn't. It would be easy to make the fill aesthetic NA by default but draw polygons underneath the path when fill isn't NA. The only issue to handle better than is currently the case would be polygons that intersect with the plot boundaries.

library(ggplot2)

set.seed(4393)
dsmall <- diamonds[sample(nrow(diamonds), 1000), ]
d <- ggplot(dsmall, aes(x, y))

# works
d + geom_contour(stat = "density_2d")

# doesn't work but should
d + geom_contour(aes(fill = stat(level)), stat = "density_2d")
#> Warning: Ignoring unknown aesthetics: fill

# works
d + geom_polygon(aes(fill = stat(level)), stat = "density_2d")

^{Created on 2018-12-21 by the reprex package (v0.2.1)}

[thomasp85]

The problem of filled contours has come up time and time again. I don’t think we should attempt to add support before ggplot2 has native support for holed polygons

[thomasp85]

I’m petty sure @hadley has opinions on this as well, if I remember correctly from the last issue that requested this

[yutannihilation]

native support for holed polygons

For reference: #2534, #2502

[clauswilke]

I've looked at the prior issues and the output returned by grDevices::contourLines(). Filled contours won't be possible with that output. I think @hadley is right that a new contouring implementation would be needed, preferably written in C or C++.

In case anybody is curious, the code for the current implementation is here:
https://github.com/wch/r-source/blob/trunk/src/main/plot3d.c
https://github.com/wch/r-source/blob/trunk/src/main/contour-common.h
It's not that long, so an adventurous person could try to figure out exactly what it does. It's not immediately obvious to me to what extent the code takes shortcuts based on the assumption that it only needs to generate lines, not closed polygons.

[clauswilke]

Instead of trying to understand the poorly documented C code in the base R distribution, it's probably easier to just implement the marching squares algorithm from scratch. A good overview is provided on wikipedia: https://en.wikipedia.org/wiki/Marching_squares

[clauswilke]

To keep the issues focused, let's take filled contours off the table for this one (#3044). Issue #2534 is about filled contours.

[clauswilke]

I realized the refactoring I proposed may not be possible. It's not clear to me how we would map levels in the contours to colors if the contours don't exist until the very end, right before drawing.

@thomasp85 @yutannihilation If you agree this would be a problem, then I'll close this issue for now. If you can see a way to work around this issue, please let me know.

[thomasp85]

I don't think that is a problem as such. The colour is directly related to the underlying data of the density/height map and really just a binning performed by the geom...

There are other issues to be solved for sure, such as what the fill of the band should correspond to (the mean, lower, or upper value of the spanned range) as well as how to achieve the current type of plot with disconnected paths, but they should all be doable...

[clauswilke]

There are other issues to be solved for sure, such as what the fill of the band should correspond to (the mean, lower, or upper value of the spanned range) as well as how to achieve the current type of plot with disconnected paths, but they should all be doable...

Yeah, I've thought through these. I think the geom could take a parameter indicating which value should be used to determine fill. The problem with the binning still concerns me, because the geom itself would have perform the color mapping. In any case I'll take a stab at this kind of a geom, but we may find that it's not appropriate for ggplot2 proper and needs to live in an extension package.

[yutannihilation]

I'm yet to figure out what the problem is...

a parameter indicating which value should be used to determine fill

Are there any way to bypass scale mapping and keep the raw fill values so that the geom can translate later?

But, I feel this is going to be hacky, considering there's no formal way for Geoms to affect on Scales. So, probably I'll agree with Claus that "we may find that it's not appropriate for ggplot2 proper."