Fix for exact specification of plot height.

neighthan commented 5 years ago

Fixes #26, fixes #25.

This changes a bit of the logic for making the plot, some of which wasn't really necessary but made it easier / less confusing for me to work on it. ratio is the reciprocal of what it was before (it's now the change in y when you move up one row in the plot). The plot is stored in reverse order compared to before when in list form (e.g. result[0] is now the bottom row of the plot instead of the top one; I reverse the rows before joining them into the final plot string to make up for this). The actual change is a rework to how we convert from the data space to the row space of the plot which makes it (IMHO) less complex (removes max2 and min2) and easy to guarantee exactly what the height is.

Caveats

I didn't understand why max2 and min2 were needed before instead of working with maximum and minimum directly, as I do in this version. Is there some drawback to the method I use here that I'm missing?
I also wasn't quite clear on when you wanted to use '┼' vs '┤'; I think that part might not be the same now. I should be able to update that quickly if you can explain to me when you want '┼' used (it seems you use it for the y-value of the first data point, which I kept, but I wasn't clear on the second condition for it).

(#28 should be merged before this one, which starts from the cleaned up Python code; I was working on a few different things in parallel, but it got to be a pain to have all the PRs completely independent. Just look at the second commit to see the changes that this PR introduces).

neighthan commented 5 years ago

This wasn't exactly intended to fix #25, but I believe it does (certainly in my tests).

coveralls commented 5 years ago

Coverage remained the same at 100.0% when pulling 30690953c7c4e73b5dde41698b1111d38b92eef1 on neighthan:height_fix into 714e0570022fdeeed910875bf374b951c1388cef on kroitor:master.

neighthan commented 5 years ago

This should give a merge conflict if #27 is merged first; doing plot_string = '\n'.join([''.join(row) for row in result[::-1]]) is the only part of that which could be easy to miss (reversing result when creating the string before adding the title).

kroitor commented 5 years ago

@neighthan hey ) Pardon for a long wait, didn't really have the time to dive into it thoughtfully (was mostly occupied with ccxt), but hope to get to it asap! Thx again for your involvement! Appreciate it very much!

gamesguru commented 1 year ago

Any updates on this?

kroitor / asciichart

Fix for exact specification of plot height. #29