metacoder icon indicating copy to clipboard operation
metacoder copied to clipboard

Thoughts on heat_tree argument formats

Open zachary-foster opened this issue 5 years ago • 4 comments

heat_tree has too many arguments for one function (>50) and there are still others I would like to add. Although this does not affect how it works, it is not great UI design. Making the arguments more modular, would give me more freedom to add things. I was thinking about different ways to split up the arguments and I came up with the following options:

Don't change

This is the easiest!

heat_tree(obj,
          node_size = n_obs,
          node_size_interval = c(5, 50),
          node_size_range = c(0.01, 0.02),
          node_size_trans = "linear",
          make_node_legend = FALSE,
          node_size_axis_label = "# OTUS",
          layout = "fr",
          initial_layout = "re",
          title = "my plot",
          title_size = 0.01,
          output_file = "my_plot.pdf")

Imitate ggpllot2's syntax somewhat

Probably the hardest to implement, but takes the least typing for the user. Documentation would be a bit harder to find and people might confuse it with ggplot2 and try to add ggplot2 things like theme (which would sometimes work, if added to the end, making it more confusing).

heat_tree(obj) +
  node_size(n_obs, interval = c(5, 50), range = c(0.01, 0.02), trans = "linear", legend = FALSE, label = "# OTUS") +
  ht_layout("fr", initial = "re") +
  ht_title("my_plot", size = 0.01) +
  ht_output("my_plot.pdf")

Using helper functions

This would be easier to implement, and be more straight-forward to document and discover the documentation.

heat_tree(
  node_size = ht_aes(n_obs, interval = c(5, 50), range = c(0.01, 0.02), trans = "linear", legend = FALSE, label = "# OTUS"),
  layout = "fr",
  initial_layout = "re",
  title = "my plot",
  title_size = 0.01,
  output_file = "my_plot.pdf")

Any thoughts @grunwald, @knausb, @Neato-Nick, @grabear? Thanks!

zachary-foster avatar Jan 16 '19 20:01 zachary-foster

I agree it's worth thinking about the best way to present all those options. The ggplot style is really nice and I think intuitive when thinking about adding layers onto graph. A + at the end of a line makes me think "And then I add this thing." But most of those heat_tree options are not additions to the plot, they're modifications. Like, ht_layout in your example would be modifying a layer already that was already placed (correct me if I'm wrong). In ggplot, most modifications to existing layers tend to fall under theme. I'm leaning towards the helper functions, since it adds more organization to setting >50 parameters while, I think, introducing less confusion. The downside of this is... I think helper functions are not friendly to new R users. I think keeping the way heat_tree parameters are currently set might be the most friendly to new users, though cluttered.

Neato-Nick avatar Jan 16 '19 21:01 Neato-Nick

Thanks for the input! I am also leaning towards helper functions at the moment.

most of those heat_tree options are not additions to the plot, they're modifications.

Yes, that is true, and is one of the reasons I worry about the ggplot2 style being confusing.

I think keeping the way heat_tree parameters are currently set might be the most friendly to new users, though cluttered.

Yes, I agree with that as well. It just makes programmers cringe. Another option is to make the helper functions optional:

heat_tree(
  node_size = ht_aes(n_obs, interval = c(5, 50), range = c(0.01, 0.02), trans = "linear", legend = FALSE, label = "# OTUS"),
  layout = "fr",
  initial_layout = "re",
  title = "my plot",
  title_size = 0.01,
  output_file = "my_plot.pdf")

or

heat_tree(
  node_size = n_obs,
  layout = "fr",
  initial_layout = "re",
  title = "my plot",
  title_size = 0.01,
  output_file = "my_plot.pdf")

zachary-foster avatar Jan 16 '19 21:01 zachary-foster

Hey @zachary-foster I've been busy as usual and I thought I had responded to this already.

I think @Neato-Nick has really good input, but I have something to add:

  • Any item that uses data available to taxa::all_names should probably be more of a helper function. This helps avoid any confusion.
ht <- heat_tree(
  node_size = ht_aes(n_obs, interval = c(5, 50), range = c(0.01, 0.02)),
  layout = "fr",
  initial_layout = "re",
  title = "my plot",
  title_size = 0.01,
  output_file = "my_plot.pdf")
...
  • For theme based items (title, node legend, edge_legend, axis_label, etc.) you could immitate ggplot2 syntax.
...
ht + 
  + node_legend(color_label, size_label)
  + edge_legend(color_label, size_label)
  + theme()
  + labs()
...
  • You could also make scale functions for the various transformations:
...
ht + 
  scale_ht_tree(color = NULL, label_size = NULL, label_color = NULL) +
  scale_ht_node(size = NULL, color = NULL, label_size = NULL, label_color = NULL) +
  scale_ht_edge(size = NULL, color = NULL, label_size = NULL, label_color = NULL)

Yes, I agree with that as well. It just makes programmers cringe. Another option is to make the helper functions optional.

Having options is good, but if it makes the coding look bad, then just stick to one strategy.

Hope I've given you something good to think about!

grabear avatar Jan 31 '19 00:01 grabear

Thanks @grabear!

Any item that uses data available to taxa::all_names should probably be more of a helper function. This helps avoid any confusion.

Yea, it would probably be less confusing to experienced users, but perhaps more confusing to new users. I am not sure. In the version I am working on now, every argument can take values from taxa::all_names, even those that don't really need to, so that at least its consistent.

I might do another rewrite in the future that will follow the gpglot2 syntax more and allow for more extensions and put it in a new package called heattree. The current version I am working on will add a lot more flexibility, but also more options. I am getting close to finishing it, so I don't want to change plans now, but in the future, a ggplot2-style syntax might be the best strategy. Under the hood, it is going that direction already, even the way the function is called is not changing much.

zachary-foster avatar Jan 31 '19 01:01 zachary-foster