Fix BarChart ignoring include_numbers in y_axis_config - #5015
Open
RizgarOzan wants to merge 2 commits into
Open
RizgarOzan wants to merge 2 commits into
RizgarOzan wants to merge 2 commits into
Conversation
DecimalNumber caches digit mobjects in a module-level dict keyed only by the digit string. Building the y-axis numbers with Text filled that cache with Text glyphs, so later tests in the same pytest worker rendered their numbers with Text instead of MathTex and failed frame comparison.
Author
|
The snapshot failures were caused by my new unit test, not the BarChart change. It built the y-axis numbers with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog
Fixed :class:
.BarChartignoringinclude_numbersiny_axis_config: y-axis numbers can now be suppressed, and are no longer added twice wheninclude_numbers=Trueis passed.Summary of Changes
Closes #3027 (the BarChart part; the
NumberLineanimation glitch mentioned later in the thread is separate and not touched here).What / Why:
BarChart.__init__never mergedy_axis_configdefaults and always calledself.y_axis.add_numbers()at the end. Soy_axis_config={"include_numbers": False}had no effect, andy_axis_config={"include_numbers": True}created the number labels twice (NumberLineadded them, thenBarChartadded them again).How: instead of just deleting the
add_numbers()call (the concern raised on the earlier attempt, #3057),BarChartnow uses a defaulty_axis_config = {"include_numbers": True}, merged with the user-supplied config the same wayx_axis_configalready is, and passes it toAxes. TheNumberLinethen adds the numbers exactly once, or not at all. Default output is unchanged:NumberLine'sinclude_numberspath callsadd_numberswith the same tick range, exclusions and font size as the bare call did.Also corrected the type hint of
Axes._update_default_configs: it declared 1-tuples althoughAxesitself calls it with 3-tuples that may containNone; mypy flagged the new 2-tuple call.How I tested:
tests/module/mobject/graphing/test_probability.py(useslabel_constructor=Text, so it runs without LaTeX). Before the fix:pytest tests/module/mobject/graphing/test_probability.py-> 2 failed (assert [VGroup(...)] == [],assert 2 == 1). After: 2 passed.pytest tests/module/mobject/graphing: 26 passed, 2 failed; the 2 failures (test_add_labels,test_NumberPlane) need LaTeX, which isn't installed on my machine, and fail the same way onmain.pre-commit run --files <changed files>: ruff, ruff format, codespell, mypy pass.tests/test_graphical_units/test_probability.pylocally (no LaTeX), so CI is the check there.Checklist