Skip to content

Clean up code: Replace exceptions, remove unused parameters, and fix type annotations in Animation, ShowPartial, Create, and DrawBorderThenFill #4214

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

irvanalhaq9
Copy link
Contributor

@irvanalhaq9 irvanalhaq9 commented Apr 7, 2025

Overview: What does this pull request change?

This pull request introduces some improvements:

  1. Replace NotImplementedError to TypeError for invalid input type in ShowPartial , Remove unused parameters and atrributes in DrawBorderThenFill, and Update return type annotation of _get_bounds() in ShowPartial and Create.
  2. Fix typo of debug message in Animation.__new__.
  3. Removes **kwargs from Animation.__init__, so it raises TypeError immediately when unknown arguments are passed.
  4. Adds _original__init__ = __init__ to Animation to prevent AttributeError when using .set_default() with no arguments.

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

Explanation Point 1:

  • Replace NotImplementedError with TypeError in ShowPartial. The docstring explicitly states it raises TypeError when the input is not VMobject.

  • In DrawBorderThenFill, parameters and attributes draw_border_animation_config and fill_animation_config are not used at all—they are just stored but never actually do anything. I double-checked with git 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 to tuple[float, float] in ShowPartial and Create. The return value is used in pointwise_become_partial, which expects two floats.

Explanation Point 2:
Only fix typo

Explanation Point 3:

  • Remove **kwargs and add use_override to Animation.__init__. So, it raises an exception (TypeError) immediately when unknown arguments are passed. Parameter use_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, running Create(Square(), rate_functions=linear) will pass silently, even though rate_functions is not a valid argument. In contrast, Mobject raises TypeError immediately when unknown arguments are passed, like Square(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 the rate_func parameter and start typing rate, the auto-completion will suggest rate_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.
  • Remove unused final_alpha_value in TransformMatchingAbstractBase as it is never read or used anywhere in the codebase (verified via git grep), and there is no documentation or comment about it. Removing it simplifies the code without affecting functionality.
  • Remove angle=TAU is also removed because in Rotating the parameter is radians, not angle, and the default value is already TAU, so no problem removing it. Removing them is consistent with this PR because they are considered unexpected keyword argument, and TypeError is raised.

Explanation Point 4:
Adds _original__init__ to Animation.
The set_default() method uses cls._original__init__ to restore the original __init__ method when no keyword arguments are passed. However, _original__init__ is not defined in the Animation base class, which causes an error when set_default() is called directly on Animation.

Links to added or changed documentation pages

Reviewer Checklist

  • The PR title is descriptive enough for the changelog, and the PR is labeled correctly
  • If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
  • If applicable: newly added functions and classes are tested

@github-project-automation github-project-automation bot moved this to 🆕 New in Dev Board Apr 7, 2025
@irvanalhaq9 irvanalhaq9 changed the title Clean up ShowPartial type check and remove unused parameters and attributes in DrawBorderThenFill and fix some return types. Clean up Animation, ShowPartial, DrawBorderThenFill and Create Apr 7, 2025
@irvanalhaq9 irvanalhaq9 changed the title Clean up Animation, ShowPartial, DrawBorderThenFill and Create Clean up Animation, ShowPartial, Create, and DrawBorderThenFill Apr 7, 2025
@irvanalhaq9 irvanalhaq9 changed the title Clean up Animation, ShowPartial, Create, and DrawBorderThenFill Clean up ShowPartial, Create, and DrawBorderThenFill Apr 7, 2025
@irvanalhaq9 irvanalhaq9 changed the title Clean up ShowPartial, Create, and DrawBorderThenFill Clean up code: Replace exceptions, remove unused parameters, and fix type annotations ShowPartial, Create, and DrawBorderThenFill Apr 8, 2025
@irvanalhaq9 irvanalhaq9 marked this pull request as draft April 9, 2025 09:06
@irvanalhaq9 irvanalhaq9 marked this pull request as ready for review April 9, 2025 10:12
@irvanalhaq9 irvanalhaq9 changed the title Clean up code: Replace exceptions, remove unused parameters, and fix type annotations ShowPartial, Create, and DrawBorderThenFill Clean up code: Replace exceptions, remove unused parameters, and fix type annotations in Animation, ShowPartial, Create, and DrawBorderThenFill Apr 9, 2025
@irvanalhaq9 irvanalhaq9 force-pushed the cleanup-showpartial-animation-config branch 2 times, most recently from 246c4ce to c68396a Compare April 20, 2025 19:36
@irvanalhaq9 irvanalhaq9 force-pushed the cleanup-showpartial-animation-config branch from 6e046fb to 3acd57a Compare April 21, 2025 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

1 participant