search

SciNim / nim-plotly

plotly wrapper for nim-lang
https://scinim.github.io/nim-plotly/
MIT License
179 stars 15 forks source link

Support subplot creation at runtime via `createGrid` #45

Closed Vindaar closed 5 years ago

Vindaar commented 5 years ago

This PR adds a Grid object and corresponding createGrid 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 the numPlots argument of createGrid can be added by using add. The given object is converted to PlotJson. A call to show on the grid, will calculate the required number of rows and columns (numPlotsPerRow is an optional argument to createGrid) and make a call to combine to get a valid PlotJson object of the subplot.

I had to move the implementation from plotly.nim to a submodule, to be able to call show from within plotly_subplots.

The Grid object has a layout field, which takes just a normal Layout object. This can also be given to createGrid, but can be set later if desired.

For some reason the show proc with filename 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. :)

brentp commented 5 years ago

I am fine with this. Seems like soon we need more general documentation now that the feature-set is expanding.

Vindaar commented 5 years ago

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... :|

Vindaar commented 5 years ago

Sorry for taking so long to get back to this. I just:

@LemonBoy: Does this look reasonable for you? :)

brentp commented 5 years ago

I haven't had a look at this. @LemonBoy PTAL?

@Vindaar is this still as you'd like it?

Vindaar commented 5 years ago

For me this is fine. I think it's ok to support both ways of adding elements to the grid. I've been using it in my own code already without issues. edit: just noticed a problem with the JS backend. Not sure yet if it's a regression on devel or introduced by me in this PR. Will investigate and report back. edit2: Sorry, done. I thought I tested the JS backend after making these changes, but apparently I didn't do that properly.