search

arafatkatze / glot

Glot is a plotting library for Golang built on top of gnuplot.
MIT License
398 stars 19 forks source link

Error when saving: This plot has 0 curves and therefore its a redundant plot and it can't be printed. #17

Open paulhovey opened 6 years ago

paulhovey commented 6 years ago

After updating to the latest commit ( 828496c875ee51ac46eab15f1e63a23050add6f4 ), I am getting an error when calling SavePlot. I believe this error is caused because nplots is 0. nplots is only updated in plotX(), plotXY(), and plotXYZ() functions, which are not being called internally anymore. This was broken on this pull request . When I manually insert those lines back in, I can plot the graph. Any reason this was taken out?

arafatkatze commented 6 years ago

@paulhovey Thanks a lot for raising the issue. @szaydel Can you please look into this and a simple pr to solve this?

Thanks

kitech commented 6 years ago

+1 the same problem. maybe broken something.

szaydel commented 6 years ago

@Arafatk: sorry about being slow to reply. Will resolve.

szaydel commented 6 years ago

@paulhovey: Would it be possible to have an example use? I am going to say that there's probably not a test for this also, which I will be sure to add.

szaydel commented 6 years ago

@Arafatk, these issues are a result of inadequate testing on my part, and premature PR: https://github.com/Arafatk/glot/pull/15. That PR relied too much on the tests that were written and not enough on the author doing functional testing beyond unit tests. I will fix things, but I can see clearly that a number of things will need to be changed in order to avoid breaking functionality that worked before https://github.com/Arafatk/glot/pull/15.

paulhovey commented 6 years ago

@szaydel testing is good for stuff under tests :) it seems as though perhaps SavePlot() isn't under test? My first thought would be to use afero as a fake file system and something like imgdiff to compare against a known good image? Would there be an easier way to test it?

szaydel commented 6 years ago

Yeah, there's much that probably needs better testing. I think there are a number of ways to do this, one might be to checksum generated plots, though I am not sure if they encode any metadata that's volatile, which would break checksumming methods. One could also compare certain offsets in the file, etc. I am not sure if a fake filesystem is required, since things could be tested against a temp location and just tossed away. But this is definitely testing for side-effects and so some thought needs to be given.