label_parse with an Empty String Shifts Labels to Incorrect Locations

[billdenney]

When a label has an empty string in it, the result appears to be shifted off by one. Note the bottom right and the bottom left in the second image below.

library(tidyverse)
library(tidygraph)
#> 
#> Attaching package: 'tidygraph'
#> The following object is masked from 'package:stats':
#> 
#>     filter
library(ggraph)

schema <-
  create_empty(n=0, directed=TRUE) %>%
  bind_nodes(data.frame(id=1:3, label=LETTERS[4:6])) %>%
  bind_edges(data.frame(from=c(1, 2, 3), to=c(2, 3, 1), label=c("", LETTERS[1:2])))

ggraph(schema) +
  geom_edge_link(aes(label=label)) +
  geom_node_label(aes(label=label))
#> Using `nicely` as default layout

ggraph(schema) +
  geom_edge_link(aes(label=label), label_parse=TRUE) +
  geom_node_label(aes(label=label))
#> Using `nicely` as default layout

Created on 2018-09-14 by the reprex package (v0.2.0).

[billdenney]

The issue appears to be that here:

ggraph/R/geom_edge.R

Line 116 in 8d23845

lab <- parse(text = as.character(lab))

The parse function when provided an empty string as part of a vector of inputs returns a shorter vector of expressions:

> parse(text=c("", "A", "B[C]"))
expression(A, B[C])
> length(parse(text=c("", "A", "B[C]")))
[1] 2

While I think that this is a difficulty in base R (reported as such: https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17471), I think that the behavior is unlikely to change, and it is probably viewed as a feature.

Something like the following may work:

lab_char <- as.character(lab)
lab <- expression()
lab[!(lab_char %in% "")] <- parse(text=lab_char[!(lab_char %in% "")])

[mmaechler]

Yes, this is definitely part of parse()s design. Note that also non-empty entries are "left away":

> Txt <- c("pi", "# A comment; the next is just empty", "       ", "1+2")
> parse(text=Txt)
expression(pi, 1+2)
> 

and as you say yourself (in the bugzilla report) this is well documented.

I think that the "mistake" is in your design that lab is given as character instead of directly as expressions. You should have your users give expressions (or 'calls' if you know the difference), as for all the regular plot/graphics functions. Users should be allowed to give either character or expressions, and there's the as.graphicsAnnot() function you should use and then pass the "graphics annotation" to the respective functions.

Both R's graphics and grid packages use as.graphicsAnnot() ... and I hope ggplot2 would too

[billdenney]

I’m confused then: If the user should directly give expression (as is the case for many other plotting functions), then they should never need to be parsed, and the label_parse argument would be superfluous. What is the rationale of that argument if not what I’m doing. (FYI, in my real use case, I’m using label_parse to generate subscripts.)

If label_parse is kept, and the preference is not to change the behavior, I think that the length of labs at the input and output of the conversion should be checked, and if it changes, that should be an error.

I would be happy to submit that as a PR.

[thomasp85]

I think Martin is referncing the approach used in ggplot2 (and hence, ggraph) more than how you try to use it.

I think a reason for the current approach is that ggplot2 works with data from dataframes and a column of calls are pretty rare. This in turn led to the current approach of specifying if a character column should be interpreted as an expression. In any way the original bug is valid in ggraph and will be fixed