Clean up code: Replace exceptions, remove unused parameters, and fix type annotations in Animation
, ShowPartial
, Create
, and DrawBorderThenFill
#4214
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.
Overview: What does this pull request change?
This pull request introduces some improvements:
NotImplementedError
toTypeError
for invalid input type inShowPartial
, Remove unused parameters and atrributes inDrawBorderThenFill
, and Update return type annotation of_get_bounds()
inShowPartial
andCreate
.Animation.__new__
.**kwargs
fromAnimation.__init__
, so it raisesTypeError
immediately when unknown arguments are passed._original__init__ = __init__
toAnimation
to preventAttributeError
when using.set_default()
with no arguments.Motivation and Explanation: Why and how do your changes improve the library?
Explanation Point 1:
Replace
NotImplementedError
withTypeError
inShowPartial
. The docstring explicitly states it raisesTypeError
when the input is notVMobject
.In
DrawBorderThenFill
, parameters and attributesdraw_border_animation_config
andfill_animation_config
are not used at all—they are just stored but never actually do anything. I double-checked withgit grep
and couldn't find any usage elsewhere. They're also unused in manimgl. So in this PR, I suggest removing them to keep things cleaner and less confusing.Verified with:
git grep output
$ git grep draw_border_animation_config manim/animation/creation.py: draw_border_animation_config: dict = {}, # what does this dict accept? manim/animation/creation.py: self.draw_border_animation_config = draw_border_animation_config $ git grep fill_animation_config manim/animation/creation.py: fill_animation_config: dict = {}, manim/animation/creation.py: self.fill_animation_config = fill_animation_config
Update
_get_bounds
return type annotation totuple[float, float]
inShowPartial
andCreate
. The return value is used inpointwise_become_partial
, which expects two floats.Explanation Point 2:
Only fix typo
Explanation Point 3:
**kwargs
and adduse_override
toAnimation.__init__
. So, it raises an exception (TypeError
) immediately when unknown arguments are passed. Parameteruse_override
is added to the__init__
, so it will not be considered as unexpected argument when passed from subclass's constructor.Reasons: Currently,
Animation
silently accepts unexpected keyword arguments without raising an error. This behavior may lead to confusion or silent bugs, since the user might think the argument is doing something, when in fact it is not used at all. For example, runningCreate(Square(), rate_functions=linear)
will pass silently, even thoughrate_functions
is not a valid argument. In contrast,Mobject
raisesTypeError
immediately when unknown arguments are passed, likeSquare(say="Hello")
.# TypeError: Mobject.__init__() got an unexpected keyword argument 'say'
NOTE:
Auto-completion for some default parameters from parent class in some animation classes, such as
Create
, cannot work properly. For example, if we want to use therate_func
parameter and start typingrate
, the auto-completion will suggestrate_functions
, which is not a parameter (but a globally imported module). Users might mistakenly think that it's a valid parameter name, but the code won’t work as intended — and the problem is: Manim does not throw an error for that.final_alpha_value
inTransformMatchingAbstractBase
as it is never read or used anywhere in the codebase (verified viagit grep
), and there is no documentation or comment about it. Removing it simplifies the code without affecting functionality.angle=TAU
is also removed because inRotating
the parameter isradians
,not angle
, and the default value is alreadyTAU
, so no problem removing it. Removing them is consistent with this PR because they are consideredunexpected keyword argument
, andTypeError
is raised.Explanation Point 4:
Adds
_original__init__
toAnimation
.The
set_default()
method usescls._original__init__
to restore the original__init__
method when no keyword arguments are passed. However,_original__init__
is not defined in theAnimation
base class, which causes an error whenset_default()
is called directly onAnimation
.Links to added or changed documentation pages
Reviewer Checklist