allow empty facet specs

[yutannihilation]

Fix #3070, fix #2986.

Summary

This PR compacts facet specs automatically (#2986) and allow empty specs (#3070).

I admit this makes the implementation of FacetGrid and FacetWrap a bit complicated in that it needs some special treatments for the case of 0-variable. But, on the other hand, accepting empty specs makes the process of building facet specs simpler and clearer. So, I believe this change is worth making.

Major changes (visible to users)

• Compact facet specs automatically.
  • If the input is NULL, treat it as an empty quosures object (quos()).
  • If the input is a quosures object, remove NULLs in it.
• Allow empty specs. When the spec is empty,
  • facet_wrap() generates a single plot with a strip. A string (all) is used for the strip label.
  • facet_grid() generates a single plot without a strip.

Internal changes

To make the major changes, I want to define the internal facet specs more strictly as following:

• FacetWrap requires a quosures object (e.g. quos(foo, bar))
• FacetGrid requires a list of two quosures objects, rows and cols (e.g. list(rows = quos(foo), cols = quos(bar)))

Accordingly, the corresponding functions below return these specs.

• grid_as_facet_list()
• wrap_as_facet_list() (new)

Under these functions, as_facet_list() is responsible for translating a spec (e.g. a quosures object, a formula, and a character) to a list of quosures. It's also responsible for the basic validation (e.g. raise an error when the spec is an uneval object), so the upper functions should always use this for the input even it's just a simple quosures object.

Note that the results of as_facet_list() are not compacted; compacting these specs is basically the role of the upper functions, grid_as_facet_list() and wrap_as_facet_list(). For convenience, a new internal function compact_facets() can be used as a short cut of flattening and compacting the spec.

Minor changes

• facet_wrap() and facet_grid() now properly rejects aes()

Example usages

library(ggplot2)

d <- data.frame(x = 1:10, g = rep(c("a", "b"), each = 5))
p <- ggplot(d, aes(x, x)) + geom_point()

# NULLs are compacted automatically
p + facet_wrap(vars(NULL, g, NULL))

# accepts empty specs
p + facet_wrap(NULL)

# accepts empty specs
p + facet_grid(NULL, NULL)

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

[clauswilke]

I think this is good in principle. Not entirely sure about the (empty) label, though. Would it be possible to just use an empty string?

[yutannihilation]

Oh, nice idea. Thanks. But, we cannot use an empty string as a variable name, so it seems I need some tweaks.

list("" = "")
#> Error: attempt to use zero-length variable name

[clauswilke]

And the label and the variable name should be the same, I assume.

So maybe (empty) is fine, or alternatively how about (all data). After all, faceting usually indicates subsets of data, and here the subset is everything.

[yutannihilation]

I forgot "(all)" is used when facet_grid(margins = TRUE). Probably we should use this label in facet_wrap() too, (and in facet_grid() too...?).

[clauswilke]

Sounds good to me.

[yutannihilation]

Two small corrections:

we cannot use an empty string as a variable name

We can, and it seems to work with the current code of ggplot2.

l <- setNames(list(""), "")
names(l)
#> [1] ""

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

"(all)" is used

(all) label is not coded in ggplot2, but is returned by reshape2::add_margins().

Anyway I'll use (all) since it would be slightly look nicer when labeller = label_both.

[yutannihilation]

Is this a typo of "uneval"...? 🤔

ggplot2/R/facet-.r

Lines 278 to 280 in 9df0a07

	if (inherits(x, "mapping")) {
	stop("Please use `vars()` to supply facet variables")
	}

[yutannihilation]

@lionel- @thomasp85 I think the proposal about empty facet specs is almost fixed, though I need to add more test cases before asking for revewing. I'd appreciate if you give some feedbacks and concerns :)

[lionel-]

The approach looks good to me!

[lionel-]

Here it's not clear whether you're mentioning two single quosure or two quosure lists. I would use "quosure lists" or "lists of quosures" instead of just the plural "quosures".

Also we generally capitalise comments. No full stop, unless there are more than one sentence.

[yutannihilation]

Thanks! Actually I'm struggling about the wording here...

Also we generally capitalise comments. No full stop, unless there are more than one sentence.

I see.

[yutannihilation]

Note that, I gave up testing strip labels ("(all)") because it seems there's no other way than visual tests, but I feel it's not worth for the cost.

[lionel-]

Looks good!

[lionel-]

I think this should be new_quosures(). The casting form converts formulas to quosures etc, and at this point we should have standardised the inputs already.

[yutannihilation]

Makes sense. Thanks!

[yutannihilation]

@thomasp85 Do you have any comments? Since this PR is mostly about facet specs and I don't think the change is big, I expect this doesn't get in the way of #3062. But, if you have any concerns, please tell me. If there's no major concern, I'll merge.

[yutannihilation]

Will merge this after the release of 3.2.

[yutannihilation]

Thanks!

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