Fix crash due to bad rounding in BarChart when no y_step was provided and the max y value was close to 0

[zeFresk]

Overview: What does this pull request change?

This PR fixes a bug in BarChart that lead to a crash if the user did not provide the y_step (i.e. tick rate, the user provided y_range=[y_min, y_max]) and the y values were very close to 0 (round(max(ys), 2) == 0).

Motivation and Explanation: Why and how do your changes improve the library?

When the user only provided the minimum and maximum of the y_range, the code for BarChart was deducing the tick rate for the y axis using the following code:

round(max(self.values) / y_length, 2)

However, this returns 0 if the maximum is small, because it is rounded down to zero. This lead to a division by zero in a np.arrange downstream. This issue can be fixed by replacing the 2 by a computed value.

This PR replaces the hard-coded 2 with $\max\left(\lceil - \log_{10}(x) \rceil, ~ 2\right)$, with $x$ being equal to max(self.values) / y_length (unrounded y_step in the original code). To ensure retro-compatibility, the newly computed value can't go below 2, in the case of large y values.

Reviewer Checklist

• The PR title is descriptive enough for the changelog, and the PR is labeled correctly
• If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
• If applicable: newly added functions and classes are tested

[JasonGrace2282]

Hey thanks for the PR! Do you mind adding a (non-graphical) test for this?

[zeFresk]

Hello and thank you for the reply. I added one simple test and checked to make sure it was failing before this PR, but don't throw errors after this PR.

If it's not enough, I could add more tests.

[JasonGrace2282]

I think it looks fine from my cursory testing :)
Could you please just fix the pre-commit linting errors, and then explain where you got your formula from (preferably as a comment in the code)?

[zeFresk]

I added comments and fixed the linting. Do you think the explanations are clear enough? I can expand them if needed.

[JasonGrace2282]

The change looks fine to me, but after trying some different combinations, it seems the y-axis is always displayed as 0, which doesn't seem very helpful.
Instead of trying to compute a new step value, what do you think about forcing the user to pass in a y_step?

[JasonGrace2282]

Did you mean to pass names to bar_names?

[zeFresk]

Yes, thank you, I will fix that.

[zeFresk]

I may have broken something with my last commits, because I recently made an animation with very low values that worked perfectly from 6 bars of values 1/10000th to 1.
I think forcing the y_step could be a good solution too. In fact I did that at first, then thought that the code I used may be useful as it fixed the problem and provided smooth transitions when animating.

Could you share some non-working examples, please? I could also try to understand what's going wrong and fix it.

[zeFresk]

# Handle cases where min and max are both positive or both negative
if x_min < x_max < 0 or x_max > x_min > 0:

to

# Handle cases where min and max are both positive or both negative
if x_min < x_max < 0 or x_max > x_min >= 0:

In the first commit (used for my renderings), I also added this change because a range from 0 to 0.000001 was not considered strictly positive or negative for some reason. I later reversed this change because it seemed to break a lot of tests. Maybe this is the cause of your troubles.

[JasonGrace2282]

Could you share some non-working examples, please? I could also try to understand what's going wrong and fix it.

from manim import *

class BarChartExample(Scene):
    def construct(self):
        values = [1 / 10000 for _ in range(8)]
        chart = BarChart(
            values=values,
            bar_names=["one", "two", "three", "four", "five"],
            y_range=[0, 2 / 10000],
            y_length=6,
            x_length=10,
            x_axis_config={"font_size": 36},
        )

        c_bar_lbls = chart.get_bar_labels(font_size=48)

        self.add(chart, c_bar_lbls)

config.preview = True
BarChartExample().render()

Is what I tried, which results in