Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP/RFC] subplot(): separate the definition of the subplots layout from the generation of the resulting plotly object #1100

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

alyst
Copy link
Contributor

@alyst alyst commented Aug 17, 2017

The PR extracts merge_plots() function from the subplot() function, so that subplot() only checks the plots argument and defines how the plots should be layed out in the merged result. The actual merging of the plotly objects is done by merge_plots() function. It enables the users to generate subplots using their own customized layouts.

Rationale

I have 2 ggplotly plots, each using facet_grid(), I want that the subplots of each ggplotly objects are interleaved in the merged result. Currently, subplot() does not support faceted ggplotly plots and does only support specifying the layout of the subplots of its arguments. E.g.:
custom_subplot

Implementation

merge_plots(plots, subplots_info, axes_info, which_layout = "merge")

subplots_info and axes_info define how the plots should be transformed into the new merged layout:

  • subplots_info enumerates all the subplots in the merged results: the source plotly object, the reference axes, the shapes and the annotations that belong to the plot.
  • axes_info enumerates all the axes of the plots and how they should be renamed and repositioned in the new layout. Repositioning of the axes automatically updates the positions of the linked shapes and annotations (via subplots_info).

also add plot "plot_index", "col" and "row" columns to the result
also unify x/y axes manipulation code
@cpsievert
Copy link
Collaborator

Thanks for the pull request. I can tell you put a lot of time into this. In the future, please open an issue before working on a pull request so we can discuss whether the idea/contribution has potential to be merged at some point.

I have 2 ggplotly plots, each using facet_grid(), I want that the subplots of each ggplotly objects are interleaved in the merged result

Solving this problem in general seem quite difficult and doesn't provide much of any new functionality. Why do you need them merged into the same plot? Is there some reason why you can't place them separate <div>s via htmltools::tagList() or crosstalk::bscols()?

@cpsievert
Copy link
Collaborator

PS. I haven't forgotten about #622. That's definitely a problem that I'm hoping to solve once I have time to properly do everything I'm wanting to do in #929. If you have interest in helping me out there, let me know and I can make some notes on things we still need done.

@alyst
Copy link
Contributor Author

alyst commented Aug 18, 2017

For my plot I needed both the interactive web version and the PDF one (so I have to use ggplot2/ggplotly, since AFAIK the free plotly.js version does not support exporting to vector formats).
To reduce both R/shiny and client webpage overhead I had to use subplots instead of generating multiple plots. Also, having multiple plotlys on a web page is not very convenient from UI usability POV.
In the end the plot code that uses the merged_plots() extension is just a few lines long; generating dynamic HTML and providing communication between the plots (to show/hide traces) would require more coding and fixing.

I had to implement it in a limited and predictable timeframe, and given my experience of reporting issues/submitting PRs to this repository (e.g. #622 is more than a year old, and had zero comments) I decided that just writing my version would be less risky for me.
Still I felt obliged to inform you and publish my PR for this nice opensource package as soon as possible. Anyway, I expected that most likely I would end up maintaining an out-of-tree branch (I hope you are fine with this).

A few comments about the state of PR. I didn't extensively test it and published the first working version, so it definitely needs some polishing.
Most likely, the interface could be further simplified by removing subplots_info argument from merge_plots() -- it could be deduced from the plots and axes_info.
Also, I admit that supporting multiple plots per plotly objects adds complexity, that's why it's implemented in a separate commit and might be omitted from the mainline inclusion.

Just to clarify, plotly is the best R package for interactive web graphics, I'm very grateful for your efforts. It's up to you to decide how to manage the community around it, I totally respect that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants