-
Notifications
You must be signed in to change notification settings - Fork 15
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
Support subplot creation at runtime via createGrid
#45
Conversation
`createGrid` returns a `Grid` object for which `[]`, `[]=` and `add` are defined, so that the user may hand any `Plot[T]` object to the `Grid`. Internally all plots are stored in a `seq[PlotJson]`.
# Note: internally the returned `Grid` object stores all plots already | ||
# converted to `PlotJson` (i.e. the `layout` and `traces` fields are | ||
# `JsonNodes`). | ||
var grid = createGrid(numPlots = 2) #, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer a rows, columns: int
and a []
/[]=
that takes a row/column pair in order to better work with MxN subplots
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally agree with you. That's what I intuitively would expect when dealing with a 2D grid anyways. @timotheecour your opinion?
I'm fine either way though.
I am fine with this. Seems like soon we need more general documentation now that the feature-set is expanding. |
Yes, I agree. I've been meaning to write some more documentation... :| |
The result of this is we can have e.g. a 2x2 grid of plots and only assign 3 without a crash. However, if we only assign (0, 0), (0, 1), and (1, 1), the plot at (1, 1) will be shifted to (1, 0)!
Previously `numPlotsPerRow` caused the calculation of the # of rows and columns to break, because we assumed `rows == 0` as a placeholder. Introduces `-1` as a special case for row or column, which means "infer from the other dimension + nPlots".
Since `parseTraces` is also useful on the JS backend, it was in addition added to `plotly_js`, since either putting it into `plotly_sugar` or creating a separate file for JS + C specific functions seemed unreasonable.
If the proc is exported its name should better reflect what it actually does
Sorry for taking so long to get back to this.
@LemonBoy: Does this look reasonable for you? :) |
For me this is fine. I think it's ok to support both ways of adding elements to the grid. |
`show` is only needed for the targets other than JS. `hasThreadSupport` is defined here again to avoid having to import it too from display.
This PR adds a
Grid
object and correspondingcreateGrid
procedure, which can be used to create subplots using a grid at runtime. It's basically what was proposed in #43.Plots can be assigned to the
Grid
object using[]=
. In addition more plots than defined by thenumPlots
argument ofcreateGrid
can be added by usingadd
.The given object is converted to
PlotJson
. A call toshow
on the grid, will calculate the required number of rows and columns (numPlotsPerRow
is an optional argument tocreateGrid
) and make a call tocombine
to get a validPlotJson
object of the subplot.I had to move the implementation from
plotly.nim
to a submodule, to be able to callshow
from withinplotly_subplots
.The
Grid
object has alayout
field, which takes just a normalLayout
object. This can also be given tocreateGrid
, but can be set later if desired.For some reason the
show
proc withfilename
arg for--threads:off
does not compile. It acts as if the proc was called, even it isn't. Not sure what's up there.I'm all ears for other ideas, features of different names for the args / objects.
@timotheecour let me know if this suits your needs. :)