ManimCommunity / manim

A community-maintained Python framework for creating mathematical animations.
https://www.manim.community
MIT License
21.42k stars 1.57k forks source link

Fix: ``NumberLine`` ``number_to_point`` broken when using ``add_tip`` #3820

Open goldenphoenix713 opened 3 months ago

goldenphoenix713 commented 3 months ago

Overview: What does this pull request change?

Fixed issue where NumberLine's number_to_point method didn't convert from number line value to image position correctly after calling add_tip after creation. See issue #3740.

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

NumberLine's number_to_point method now converts from number line value to image position correctly after calling add_tip.

Links to added or changed documentation pages

No documentation changed.

Further Information and Comments

Also fixed was a small issue with the final tick mark of a number line not always appearing when a tip was added.

Reviewer Checklist

goldenphoenix713 commented 3 months ago

There's an issue now where the expected values in several of the tests (the ones that depend on NumberLine or classes that implement NumberLine, e.g., Axes don't match the new values computed whenever there's a n2p or p2n method called. Since the calculated values are, in fact, different due to the bug fix, the expected values in the tests will need to be changed to reflect the new calculated values. Please advise on how to proceed.

JasonGrace2282 commented 3 months ago

Hello! Thanks for taking on this issue, and helping out with developing Manim :)

Since the calculated values are, in fact, different due to the bug fix, the expected values in the tests will need to be changed to reflect the new calculated values. Please advise on how to proceed.

You can see how to regenerate the graphical data in our docs

However, I would think carefully before you do, and look at the difference between your results and the current test data. E.g. for the test tests/test_graphical_units/test_coordinate_systems.py::test_plot_surface, using pytest --show_diff tests/test_graphical_units/test_coordinate_systems.py::test_plot_surface gives me this

Some possible options to explore is a boolean parameter to toggle between get_start/base, or if you think of anything better feel free to try that out too :)

goldenphoenix713 commented 3 months ago

I think one possible way forward is to have an input parameter that control whether the arrow tip isn't create insides or outside of the end of the line. The default value of this parameter can be the original implementation (with the tip generated inside the end of the line), but it would also allow users to configure the arrow tips to their liking. By setting the default to the tip inside, the tests (hopefully) should generate data that matches the expected values.

My guess is we'd have to add this new parameter to all classes that use / are subclasses of TipableVMobject