search

SciNim / nim-plotly

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

make plotly work using WSL #61

Closed Vindaar closed 4 years ago

Vindaar commented 4 years ago

This PR addresses #59 by adding support for WSL.

Note that this does assume the user wants to show the plots in a Windows browser! I haven't set up a browser I can open from WSL itself, so I'm not able to see if / how that might work. Under those circumstances the actual posix approach should work (which isn't possible after this PR). One could introduce some command line argument to specify whether to use a Windows or WSL browser.

What do you think @Clonkk?

Vindaar commented 4 years ago

Ok, I just rebased this on the other PR (so that CI has a chance of passing) and moved the show calls for the suplot grids to the main display file. Finally added some tests that show that indeed the calls to show/saveImage fail if compiled without --threads:on.

Clonkk commented 4 years ago

I'll check this out tomorrow but it looks good.

Clonkk commented 4 years ago

So the WSL problem has been solved (it correclty opens on a Windows browser).

saveImage with --threads:on seems to never return.

Vindaar commented 4 years ago

saveImage with --threads:on seems to never return.

Hm, that's weird. That's exactly why I switched the websocket library. You did check out this full branch, correct? So the websocket library being used is ws instead of websocket?

Clonkk commented 4 years ago

Yup you're right that's my bad, it's working as intended. What happend is that I forgot to uninstall plotly from nimble before using nimble develop on the repo I cloned & checked out.

Vindaar commented 4 years ago

And this can go in too now.

I'm not entirely happy with the /proc/version check for each plot, but I don't want to introduce a global for this. One could check at compile time, but it seems wrong to reduce compatibility of the binary because of that. So I propose to leave it like this for the time being.

brentp commented 4 years ago

not sure what is going on with these checks.

Vindaar commented 4 years ago

No idea either. Can you just merge regardless (I can't do anything) or do you somehow override them on travis directly?

brentp commented 4 years ago

i can merge despite the non-pass checks on github with my admin privileges.