add `edge_label` and `node_label` arguments?

Question

add `edge_label` and `node_label` arguments?

mattansb opened this issue 4 years ago · 10 comments

Okay, seeing as how the graph_sem(..., label = <something>) is passed (it seems) to both get_edges() and to get_nodes(),

I think having edge_label and node_label (lazily evaluated in get_edges() and to get_nodes()) as argument in graph_sem() and in prepare_graph().

So this:

graph_sem(fit, 
          edges = get_edges(fit, label = paste(est_std, condint_std, sep = "\n"), 
          nodes = get_nodes(fit, label = "name"))

Becomes this:

graph_sem(fit, edge_label = paste(est_std, condint_std, sep = "\n"), node_label = "name")

Which is much cleaner IMO 😄

Answer 1 · 2021-03-06T14:23:59.000Z

I don't see this happening, as it is opposed to the philosophy that each function should perform one clearly defined task.

Answer 2 · 2021-03-06T14:24:20.000Z

The more likely development is that the label argument will disappear from graph_sem()

Answer 3 · 2021-03-06T14:31:55.000Z

This would also mean graph_sem.lavaan() would fall out of favor any time anything (even the simplest) would be needed to change, right?

I mean if I'm already:

graph_sem(fit, 
          edges = get_edges(fit, label = "est_std"), 
          nodes = get_nodes(fit, label = "name"))

I might as well

graph_sem(edges = get_edges(fit, label = "est_std"), 
          nodes = get_nodes(fit, label = "name"),
          layout = get_layout(fit))

No?

I'd say that the most common thing people want to "control" are the labels - I think then that it merits considering this, for ease of use...

Answer 4 · 2021-03-06T14:36:41.000Z

It's worth considering, but this kind of change to the user interface is not easy to revert; undoing it would mean supporting the legacy interface during a phase-out period, adding notifications that the interface is changing, etc. I'm very reluctant for that reason.

Answer 5 · 2021-03-06T14:37:17.000Z

That's also why the label argument is currently still supported, even though I'd rather not have it - it would break backward compatibility

Answer 6 · 2021-03-06T14:42:21.000Z

I will ditch the label argument in my teaching this year (starting this week!) hope I can convince you by next year (:

Thanks Casper for all your work!! 🎉🎉🎉🎉🥇🥇

Answer 7 · 2021-03-06T14:54:25.000Z

No problem mate, debugging is a huge task so I should thank you! Remember my offer to get on board as a package author, we could discuss these interface changes on equal footing. I'm leaving this issue as an enhancement request.

Answer 8 · 2021-03-06T16:20:02.000Z

BTW, instead of the syntax you wrote in the initial post, I would suggest the following makes more sense:

prepare_graph(fit) %>%
  edit_graph({label = paste(est_std, confint_std)}) %>%
  edit_graph({label = name}, "nodes") %>%
  plot()

Answer 9 · 2021-03-07T09:14:26.000Z

I've documented this in the plotting graphs vignette to make it more clear.

Answer 10 · 2021-03-07T10:32:32.000Z

Thanks!

I think #26 can also help resolve a similar "need" - to streamline the plotting (in one pipe?).