search

alandefreitas / matplotplusplus

Matplot++: A C++ Graphics Library for Data Visualization 📊🗾
https://alandefreitas.github.io/matplotplusplus/
MIT License
4.1k stars 311 forks source link

Fix polarhistograms with many bins #358

Closed j0hnnybash closed 1 year ago

j0hnnybash commented 1 year ago

This PR consists of two fixes which both affected the correct display of polarhistograms with more than 89 bins. Note: The second commit also increases the minimum polarhistogram resolution from 89 to 359 line segments.

Fix 1: Fix linspace(a,b,1) resulting in NaN element

This underlying issue caused polarhistograms with more than 89 bins to not be displayed at all.

Previously passing n=1 to linspace resulted in a one element vector containing NaN. With this change it will result in a one element vector containing the first ("lower") bound. This is the natural solution given the current implementation and matches the behavior of numpy.linspace, unfortunately however it does not match the behavior of MATLAB linspace which returns the "upper" bound instead.

Fix 2: Ensure at least two points (one circle segment) is used per polarhistogram petal

Change petal resolution to the larger one of (a) 360 points (359 segments) per circle or (b) n_histogram_bins+1 points (n_histogram_bins segments). Ensuring at least one segment per bin petal.

Previously polarhistograms had a fixed resolution of 90 points (89 segments) per circle. Specifying more than 89 bins would result in either an empty plot (due to a bug in linspace, see Fix 1) or too narrow petals (with the linspace bug fixed).

alandefreitas commented 1 year ago

This is the natural solution given the current implementation and matches the behavior of numpy.linspace, unfortunately however it does not match the behavior of MATLAB linspace which returns the "upper" bound instead.

Yes. That's reasonable. Even though matlab was the reference for both implementations. In C++, both functions are kind of silly and should actually be some polyfill for std::ranges::views::iota or some variantion of that but we don't have that right now anyway.

Change petal resolution to the larger one of (a) 360 points (359 segments) per circle or (b) n_histogram_bins+1 points (n_histogram_bins segments).

Yes. These precision changes are OK to apply. The proper solution would be dynamic precision based on figure properties but gnuplot (gnuplot again) through pipes doesn't give us that level of control. Until some SDL backend solution comes up, adjusting precision on user needs is the best we can do and it's very reasonable.

j0hnnybash commented 1 year ago

I updated the commit messages to match the projects convention (as requested here).