-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add docstring for Mobject.rotate
and Refactor Rotating
and also add its docstring
#4147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Mobject.rotate
and Rotating
class
b93569b
to
7edb2c3
Compare
58fce57
to
82a186c
Compare
b29cadf
to
f800288
Compare
Mobject.rotate
and Rotating
classMobject.rotate
and Refactor Rotating
and also add its docstring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I like the changes in general.
Honestly, I'm not sure whether Rotating
should even exist in the first place, as it's very similar to Rotate
with a hardcoded rate_func=linear
and run_time=5
(now run_time=2
). However, that's a discussion for another day.
I do have one specific question:
about_point: np.ndarray | None = None, | ||
about_edge: np.ndarray | None = None, | ||
run_time: float = 5, | ||
run_time: float = 2, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason behind this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to 2 seconds because 5 seconds felt unnecessarily long. As far as I can tell, there wasn’t a specific reason for choosing 5 seconds. Most animation classes in Manim default to 1 or 2 seconds (like DrawBorderThenFill
, Write
, FocusOn
, Wiggle
, ApplyWave
, LaggedStartMap
), so 2 seconds feels more consistent.
I'm also fine with keeping it at 5 seconds if that’s the preferred default — just let me know.
Overview: What does this pull request change?
Mobject.rotate()
.Rotating
, and add detailed docstring, and adjust Rotating parameter in Example Gallery.Breaking change:
radians
parameter toangle
to be consistent with the.rotate()
and theRotate
class.Rotating
to be consistent with the.rotate()
and theRotate
class.Before:
Rotating(mobject, axis, radians, ...)
After :
Rotating(mobject, angle, axis, ...)
Links to added or changed documentation pages
rotate
: https://manimce--4147.org.readthedocs.build/en/4147/reference/manim.mobject.mobject.Mobject.html#manim.mobject.mobject.Mobject.rotateRotating
: https://manimce--4147.org.readthedocs.build/en/4147/reference/manim.animation.rotation.Rotating.html#manim.animation.rotation.RotatingReviewer Checklist