search

fslaborg / FSharp.Charting

Charting library suitable for interactive F# scripting
http://fslab.org/FSharp.Charting/
Other
216 stars 67 forks source link

Chart.Save causes issues from service #104

Closed mathias-brandewinder closed 8 years ago

mathias-brandewinder commented 8 years ago

In a recent project, I needed to create and save charts from a windows service. This causes issues because - as far as I understand - Chart.Save temporarily pops a window. With some help from @tpetricek I ended up addressing the problem this way:

https://github.com/mathias-brandewinder/worldbankbot/blob/master/WorldBankBot/WorldBankBot/Twitter.fs#L157-L166

Perhaps refactoring Chart.Save using that approach might be a good idea? I don't know if the problem is important enough, and if it has implications outside of Windows. Happy to send a PR if someone can confirm it works outside of Windows.

tpetricek commented 8 years ago

This part of FSharp.Charting is Windows-only anyway. I think changing Chart.Save so that it does not open & close window is a good idea in general - if you can send a PR, that would be great!

(As far as I can see from the history, we didn't have it that way originally - so it's not that we changed the code to show the dialog in order to fix some bug.)

/cc @simra who did some work on this part, just in case he has some thoughts!

mathias-brandewinder commented 8 years ago

Cool, thanks for the clarification re: Windows - I'll also give some thought as to whether we might add optional arguments to save, then - for instance, setting the size, as in this case.

simra commented 8 years ago

HI, sorry for the delay. Yes, Chart.Save was an added convenience method because a window was needed to successfully render in Windows. I added it from a sample provided by another user but was never really happy with the call because it left no room for window size and other useful optional parameters. I looked at it once to see if we could give it optional parameters but I think the blocking issue was that it's not an object method but a static class method. Definitely worth re-visiting to see if we can make that interface better.

surban commented 8 years ago

Removing the window popup during save would also be helpful to users plotting 100+ figures in a script. Having constant window popups of very short duration makes the computer unusable during plotting.

simra commented 8 years ago

Since the window is necessary (on Windows), the right thing to do here is to detect and re-use the existing pop-up. It will take a little resource management. Since the function is a helper we could put a small burden on the user to own the window and dispose of it when done. Thoughts?

tpetricek commented 8 years ago

FsLab oes this without actually showing a window: https://github.com/fslaborg/FsLab/blob/master/src/FsLab.Runner/Formatters.fs#L209

I think that approach would work here too.

simra commented 8 years ago

That could work and I'll try to put together a pull request. The one outstanding issue is that it's still not very performant in the cases you have a lot of charts to generate. A hidden reusable control would be ideal if we could figure out the question of managing that resource.

simra commented 8 years ago

I'll leave the PR open a few days for folks to have a look. One remaining open issue with this function is wanting to pass optional parameters like size, file format, etc.

simra commented 8 years ago

Merged the PR. Ping me if you see any issues.