manim
manim copied to clipboard
Coords to point accept sequence and allow omitted positions
Changelog / Overview
Fixes #1666
- Allows coords_to_point to accept sequences
- Adds padding for mobject's shift method when less than 3 dimensions are set
Motivation
Some type hints in coordinate systems said they accept sequences but only vargs were accepted, fixed typehints and allow sequences to be passed.
Explanation for Changes
Gives more flexibility for coords_to_point method while keeping it backwards compatible. Also fixes type hints
Testing Status
Further Comments
I am not sure of all the places in manim where coordinates are accepts but it forces you to state all three x, y and z dimensions. I am guessing this only addresses a small part of it. I think it would be better to create a class with x, y, z or something to be used across manim to keep it consistent
Checklist
- [X] I have read the Contributing Guidelines
- [X] I have written a descriptive PR title (see top of PR template for examples)
- [X] I have written a changelog entry for the PR or deem it unnecessary
- [X] My new functions/classes either have a docstring or are private
- [X] My new functions/classes have tests added and (optional) examples in the docs
- [X] My new documentation builds, looks correctly formatted, and adds no additional build warnings
Reviewer Checklist
- [ ] The PR title is descriptive enough
- [ ] The PR is labeled correctly
- [ ] The changelog entry is completed if necessary
- [ ] Newly added functions/classes either have a docstring or are private
- [ ] Newly added functions/classes have tests added and (optional) examples in the docs
- [ ] Newly added documentation builds, looks correctly formatted, and adds no additional build warnings
In general, a great idea! Currently, I think it is a bit confusing if shift is working with only 2 parameters, and move_to still requires 3.
class Example(Scene):
def construct(self):
d=Dot(point=[1,2])
d.shift([-3,0])
d.move_to([4,-3]) <- not working
self.add(d)
I made an issue https://github.com/ManimCommunity/manim/issues/539 for that some time a go, which I am closing now, as we can also have here. I think we should also make some careful speed testing for this new feature, as it might slow manim down.
In general, a great idea! Currently, I think it is a bit confusing if shift is working with only 2 parameters, and move_to still requires 3.
class Example(Scene): def construct(self): d=Dot(point=[1,2]) d.shift([-3,0]) d.move_to([4,-3]) <- not working self.add(d)
I made an issue #539 for that some time a go, which I am closing now, as we can also have here. I think we should also make some careful speed testing for this new feature, as it might slow manim down.
Yes and I imagine there is quite a few places in manim that will still require 3 coords, there was a discussion about this on discord and I think rather than plugging holes in different methods to ensure they accept less than 3 axis, we could use a Point class and begin using that across manim. It will be a bigger task but easier to make things like this consistent across all the methods
Might be related to
- #2739
@ryanmccauley211 do you want to test if this also fixes #1666 ? Then we could close this or move missing changes to a new PR?!