Split geom_hline into geom and annotation (fixes #426)

[wch]

This fixes #426. As discussed, it splits geom_hline into a geom and annotation.

If this looks like the right approach, I can do the same for vline and abline. If so, should I put annotate_hline, annotate_vline, annotate_abline all in one file since they're quite simple? Or they could be added to their respective geom-Xline.r files.

Also, some of the visual tests will have to be updated.

[wch]

It also looks like there's a lot of code in the stat_Xline functions that's intended to remove duplicate lines. Can I remove that stuff? It would simplify the code a lot.

Also, stat_abline gives default values for slope and intercept. Is that necessary?

[wch]

The code for the line stats is a bit simpler with the duplicate stuff removed. It might be a good idea to print a message when using a geom with fixed intercept values, though, because it's easy to do accidentally. I also removed the default values for StatAbline.

Here's some code that tests what should be expected when the duplicate line prevention code is removed:

p <- ggplot(mtcars, aes(x = wt, y = mpg)) + geom_point()

p + geom_abline(aes(intercept = mpg, slope = mpg/5), alpha = .2) # Lots of lines
p + geom_abline(intercept = 20, slope = 1, alpha = .2)      # Should be dark
p + geom_abline(aes(intercept = 20, slope = 1), alpha = .2) # Should be dark
p + annotate_abline(intercept = 20, slope = 1, alpha = .2)  # Should be light


p + geom_vline(aes(xintercept = wt), alpha=.2)   # Lines through each point
p + geom_vline(xintercept = 3, alpha=.2)         # Should be dark
p + geom_vline(aes(xintercept = 3), alpha=.2)    # Should be dark
p + annotate_vline(xintercept = 3, alpha=.2)     # Should be light


p + geom_hline(aes(yintercept = mpg), alpha=.2)   # Lines through each point
p + geom_hline(yintercept = 20, alpha=.2)         # Should be dark
p + geom_hline(aes(yintercept = 20), alpha=.2)    # Should be dark
p + annotate_hline(yintercept = 20, alpha=.2)     # Should be light

[hadley]

Looks good! I wonder now if the stat functions could be removed altogether and just moved into the geoms? I'm trying to remember why I had separate stat functions - it's probably something to do with either correctly setting scale limits, or with non-Cartesian coordinates. If you have tests for those and they work fine, I think we could go ahead and remove the stat functions.

[hadley]

And we might not want to include this in 0.9.1 since it breaks backward compatability

[hadley]

To reduce duplication (esp. with annotate) maybe the the layer construction should be extracted out into its own function?

[hadley]

Or should annotate have special cases for vline, hline, and abline so that you can just use annotate?

[wch]

I was thinking that one way to go would be to simply call annotation(), but I think it would require some modification. Is there a reason why annotation() looks specifically for x, xmin, and xmax, but not other values, like xintercept?

[hadley]

I probably just forgot xintercept and yintercept - the other problem is that they don't currently work with stat_identity, but maybe that can go away as per my other comment. The reason that position aesthetics are treated differently is that they should be scaled (i.e. you specify position in data coordinates), but all other aesthetics should be treated as set parameter values (e.g. colour = "red" shouldn't be scaled).

That said, there were a couple of bug reports on the mailing list about annotations not working with various scales.

[wch]

Regarding removing the stat: it apparently also can work if passed a function for intercept. I think if that capability were removed, it should be simple to move the remaining code into the geom.

[hadley]

I now think that should be removed - it's not elegant, and it's not inline with the way that the rest of ggplot2 works.

[wch]

OK, I removed the stat and the examples I posted above still work. The code is much simpler now.

What remains to (possibly) be done:

• Merge help pages
• Move annotate_Xline functions into annotate()

[wch]

Another thought: as long as we're breaking compatibility, all these geoms could probably be collapsed into a single one, like geom_line.

It could take xintercept, yintercept, and slope. The xintercept and yintercept would be incompatible, but you could use slope with either x or y intercept.

[hadley]

I deliberately made it three separate functions because I think it makes more sense to keep behaviours separate, especially when you start having combinations of arguments that don't make sense.

[hadley]

I'd prefer data$y <- yintercept %||% data$yintercept

[BrianDiggs]

So a horizontal line would be specified with yintercept=... and slope=0, a vertical line with xintercept=... and slope=Inf? Could the value of slope be inferred if missing and only one of xintercept or yintercept is specified (the assumption being if no slope is specified, a horizontal or vertical line is desired)?

[wch]

@BrianDiggs I was thinking of having slope default to NULL, and infer whether it's horizontal or vertical by yintercept or xintercept is present.

Come to think of it, you can always specify a line with two of the three: xintercept, yintercept, and slope. And if you just have xintercept or yintercept, you can assume that it's vertical or horizontal.

Here's what combinations would be valid:

xintercept	yintercept	slope	valid?
yes	yes	yes	no
yes	yes	no	yes
yes	no	yes	yes
yes	no	no	yes
no	yes	yes	yes
no	yes	no	yes
no	no	yes	no
no	no	no	no

[hadley]

Yes, this is the way that abline works in base R - and I don't like it. If you need a table to figure out what the valid combination of arguments is, I think there's something wrong with your function!

[wch]

I've consolidated the docs into geom-hline.r, and clarified them a bit.

There's one inconsistency between annotate form of abline and the others. With hline and vline, you can pass a vector as the intercept values, but with abline, you can't:

# Works
p + annotate("hline", yintercept = seq(10, 30, by = 5))
p + annotate("vline", xintercept = 1:5)

# Doesn't work
p + annotate("abline", intercept = c(17, 22), slope = c(0.5, 1))

I think this is because annotate() captures xintercept and yintercept explicitly and puts them in a list that serves as the data passed to geom_xxx, but it doesn't capture intercept and slope, so they just get passed as ... args, which are expected to be single values.

In general, I like the idea of being able to pass vectors as aesthetic properties using annotate -- then it could be used to draw any sort of geom based on vector values. This may require some retooling of how it works, though.

[hadley]

Oh hmmm, good catch. The problem in general is that putting other aesthetics in the data frame will cause them to be mapped - maybe we need some way to turn that off? (But that seems like it would add a lot of complexity for little gain)

[wch]

Could we just rep all the passed-in vectors to be the same length as the longest vector, and then put them all into a data frame? That way, none of the aesthetics would have to hard-coded into annotate(). Are values for ... used for anything else besides aesthetic properties?

[hadley]

But how would you stop them from being mapped? i.e. if you do annotate("text", label = "My label", colour = "blue") you don't want the scale to map blue to something else.

[hadley]

The bug with geom_abline and multiple slopes/intercepts is probably related to #31

[wch]

Regarding stopping vars from being mapped, I can't see a way to avoid it right now. What if there were an option to pass a "raw" (unmapped) data frame to layer()?

[hadley]

I think that's something I explored a bit in the layers repo - lets not worry about it for now.

[hadley]

Could you please rebase/merge against master, re-document with the development version of roxygen2 (install_github("klutometis/roxygen) and resubmit?

[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/