SciNim / nim-plotly

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

Add ability to save plots as png, jpg, svg, webp #18

Closed Vindaar closed 6 years ago

Vindaar commented 6 years ago

This commit is introduces the ability to save the plots created by plotly directly to file. Normally one needs to use the plotly user interface to save the plots. This is

  1. inconvenient if one needs to store more than one or two images
  2. it only allows to store the images as png.

With this somewhat complicated workaround, we can save the images as

It works by injecting a few more lines of Javascript code into the HTML template, which uses Plotly.toImage to create a data packet of the desired size and filetype from the plot. We then open a websocket connection and send the image data to a websocket server, which we start before we open the browser. That server (see image_retrieve) decodes the image data and saves it to file.

The downside to this implementation is that we have to compile plotly with --threads:on.

Maybe there's an easier way to accomplish this, but I wanted to have the plots as svg files and this was the only way I know to make it work. :)

edit: and of course the the module now depends on websocket.

edit2: ok, this is still problematic. I'm struggling to get it to work properly when there are several plots being created one after another... I'll get back to it in a few days probably.

Vindaar commented 6 years ago

Ok, that took a lot longer than I wanted, but it seems to be working now. Might still be some issues left. :)

I have way too little experience with the async module... Or I should just properly read the manual cough.

edit: Ok, I don't quite know what the problem with travis is and how to fix it. I guess it's probably the whole websocket talking etc? Not sure if example 12 should maybe just be left out of the tests.

brentp commented 6 years ago

this would be very useful!

Vindaar commented 6 years ago

So it seems to be working fine on my end now. The annoying issue right now is that if one compiles a project using plotly without thread support the compiler message isn't very helpful. Because instead of the typical

Error: Threadpool requires --threads:on option.

one is greeted by

plotly/image_retrieve.nim(46, 20) Error: undeclared identifier: 'Channel'

which for the average user is about as helpful as... no error at all.

I'll try to do two things in the next few days:

  1. catch whether the user compiles with --threads:on and only implement the save image functionality only in that case.
  2. create an issue at the Nim repo / see if I can improve the error message if Channel is being used without threading support and create a PR.
Vindaar commented 6 years ago

Ok, I now put the access to the save image feature behind a guard checking for the --threads:on compile flag. If it's not set, the user sees a compiler error with a message that this flag is needed.

edit: and I changed the all.nim file again. All 11 other examples are still compiled single threaded, but 12 as multi threaded.

Vindaar commented 6 years ago

Travis fails because example 12 hasn't stopped after 10 minutes. Well, that's expected, because there is no websocket client sending any data to the server. I don't know whether there is some way to make this work. Otherwise I'd just remove the multithreaded tests from travis.

brentp commented 6 years ago

let's just not run example12 in travis-ci though I think there may be a way to have travis run a browser for testing...

ghost commented 6 years ago

@brentp there's - look at nim forum

Vindaar commented 6 years ago

Ok, after probably 20 travis build failures, I finally figured out how to make all tests run in a browser.

For some reason travis has some really weird behavior. It picks google chrome despite me only adding firefox as an addon and the xdg-open call is blocking.

I replaced docker by choosenim and run firefox in a virtual X server using xvfb. In order to get non-blocking behavior for xdg-open I introduced an optional -d:travis flag, which if set will replace Nim's openDefaultBrowser by a version which always appends an ampersand. I didn't want to append & to the two calls instead, because

  1. it's really not needed when running plotly locally
  2. no idea what other effects it might have.

@brentp Please take a look. :)

brentp commented 6 years ago

now it has a conflict with the other PR I just merged. I am just sniping issues today but can resolve this tomorrow if you don't beat me to it.

Vindaar commented 6 years ago

Don't worry, I'll rebase it onto the current master later today. :)

Vindaar commented 6 years ago

Ok, rebasing turned out to be too ugly. Merged master instead.

brentp commented 6 years ago

thanks!