-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Improve FadeIn and FadeOut classes with error handling and type safety #4308
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,10 +20,9 @@ | |
] | ||
|
||
import numpy as np | ||
from typing_extensions import Any | ||
from typing import Any, Union | ||
|
||
from manim.mobject.opengl.opengl_mobject import OpenGLMobject | ||
|
||
from ..animation.transform import Transform | ||
from ..constants import ORIGIN | ||
from ..mobject.mobject import Group, Mobject | ||
|
@@ -56,25 +55,44 @@ | |
scale: float = 1, | ||
**kwargs: Any, | ||
) -> None: | ||
|
||
if not mobjects: | ||
raise ValueError("At least one mobject must be passed.") | ||
raise ValueError("At least one mobject must be provided for fading.") | ||
|
||
for mob in mobjects: | ||
if not isinstance(mob, Mobject): | ||
raise TypeError(f"Expected Mobject instances, got {type(mob)}") | ||
|
||
mobject = mobjects[0] if len(mobjects) == 1 else Group(*mobjects) | ||
|
||
self.point_target = False | ||
|
||
if shift is None: | ||
if target_position is not None: | ||
if isinstance(target_position, (Mobject, OpenGLMobject)): | ||
target_position = target_position.get_center() | ||
shift = target_position - mobject.get_center() | ||
elif not isinstance(target_position, np.ndarray): | ||
raise TypeError( | ||
"target_position must be a Mobject or np.ndarray" | ||
) | ||
shift = np.array(target_position) - mobject.get_center() | ||
self.point_target = True | ||
else: | ||
shift = ORIGIN | ||
else: | ||
if not isinstance(shift, np.ndarray): | ||
raise TypeError("shift must be of type np.ndarray") | ||
|
||
if not isinstance(scale, (int, float)): | ||
raise TypeError("scale must be a number") | ||
|
||
self.shift_vector = shift | ||
self.scale_factor = scale | ||
|
||
super().__init__(mobject, **kwargs) | ||
|
||
def _create_faded_mobject(self, fadeIn: bool) -> Mobject: | ||
"""Create a faded, shifted and scaled copy of the mobject. | ||
"""Create a faded, shifted and scaled copy of the mobject. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indentation error |
||
|
||
Parameters | ||
---------- | ||
|
@@ -86,16 +104,24 @@ | |
Mobject | ||
The faded, shifted and scaled copy of the mobject. | ||
""" | ||
faded_mobject: Mobject = self.mobject.copy() # type: ignore[assignment] | ||
|
||
faded_mobject = self.mobject.copy() # type: ignore[assignment] | ||
|
||
if not isinstance(faded_mobject, Mobject): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check is redudant because root-Class does not allow content to be anything else than Mobject. |
||
raise RuntimeError("Failed to create faded mobject copy.") | ||
|
||
faded_mobject.fade(1) | ||
|
||
direction_modifier = -1 if fadeIn and not self.point_target else 1 | ||
|
||
faded_mobject.shift(self.shift_vector * direction_modifier) | ||
faded_mobject.scale(self.scale_factor) | ||
|
||
return faded_mobject | ||
|
||
|
||
class FadeIn(_Fade): | ||
r"""Fade in :class:`~.Mobject` s. | ||
r"""Fade in :class:`~.Mobject` s. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indentation error |
||
|
||
Parameters | ||
---------- | ||
|
@@ -143,7 +169,7 @@ | |
|
||
|
||
class FadeOut(_Fade): | ||
r"""Fade out :class:`~.Mobject` s. | ||
r"""Fade out :class:`~.Mobject` s. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indentation error |
||
|
||
Parameters | ||
---------- | ||
|
@@ -187,5 +213,9 @@ | |
return self._create_faded_mobject(fadeIn=False) | ||
|
||
def clean_up_from_scene(self, scene: Scene) -> None: | ||
"""Remove the mobject from the scene after fading out.""" | ||
super().clean_up_from_scene(scene) | ||
self.interpolate(0) | ||
try: | ||
self.interpolate(0) | ||
except Exception as e: | ||
raise RuntimeError(f"Error during cleanup interpolation: {e}") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this really add value? |
Check notice
Code scanning / CodeQL
Unused import Note