Adding type annotations to manim.utils.* #3999

henrikmidtiby · 2024-11-04T21:05:04Z

Overview: What does this pull request change?

The PR adds type annotations to some of the elements in manim/utils

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

Type annotations are a good thing.

Further Information and Comments

I have chosen to go for smaller PR regarding adding type annotations.
The endeavor of #3981 is a bit frightening...

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

manim/utils/bezier.py

-def inverse_interpolate(start: float, end: float, value: Point3D) -> Point3D: ...
+def inverse_interpolate(
+    start: float, end: float, value: InternalPoint3D
+) -> InternalPoint3D: ...


manim/utils/bezier.py

-def inverse_interpolate(start: Point3D, end: Point3D, value: Point3D) -> Point3D: ...
+def inverse_interpolate(
+    start: InternalPoint3D, end: InternalPoint3D, value: InternalPoint3D
+) -> Point3D: ...


manim/utils/color/core.py

manim/scene/scene_file_writer.py

manim/utils/simple_functions.py



-def clip(a, min_a, max_a):
+@overload
+def clip(a: float, min_a: float, max_a: float) -> float: ...


manim/utils/simple_functions.py

+
+
+@overload
+def clip(a: str, min_a: str, max_a: str) -> str: ...


henrikmidtiby · 2024-11-05T17:10:02Z

@JasonGrace2282 Would you take a look at this PR?

JasonGrace2282 · 2024-11-05T17:38:18Z

I'll take a look when I find the time.

manim/utils/family_ops.py

manim/utils/caching.py

manim/utils/module_ops.py

 __all__ = ["scene_classes_from_file"]


-def get_module(file_name: Path):
+def get_module(file_name: Path) -> types.ModuleType:


…mages.py

chopan050

Wow, this is a huge PR! It will take a long time to review. I added 47 comments and I'm not finished yet, but I hope to continue reviewing soon.

A new PR (#4027) has been merged. This renames InternalPoint3D (the NumPy array) to Point3D, and Point3D (anything resembling a 3D point) to Point3DLike. Something similar happened with the other type aliases. You might wanna take a look at it.

chopan050 · 2024-12-23T04:00:51Z

manim/_config/utils.py

@@ -692,7 +692,7 @@ def digest_parser(self, parser: configparser.ConfigParser) -> Self:

        return self

-    def digest_args(self, args: argparse.Namespace) -> Self:
+    def digest_args(self, args: argparse.Namespace | list[str]) -> Self:


args cannot be a list[str] in this case. args is an object with multiple attributes which a list doesn't have, as it can be seen in lines such as:

if args.file.suffix == ".cfg": args.config_file = args.file

Suggested change

def digest_args(self, args: argparse.Namespace | list[str]) -> Self:

def digest_args(self, args: argparse.Namespace) -> Self:

chopan050 · 2024-12-23T04:21:40Z

manim/_config/utils.py

@@ -1395,7 +1395,7 @@ def renderer(self, value: str | RendererType) -> None:
        self._set_from_enum("renderer", renderer, RendererType)

    @property
-    def media_dir(self) -> str:
+    def media_dir(self) -> str | Path:


Line 29 in manim/cli/render/output_options.py describes how the --media_dir option is parsed:

option( "--media_dir", type=Path(), default=None, help="Path to store rendered videos and latex.", ),

which means it will be always parsed as a Path. Thus, this property should return and receive a Path.

Technically, all those properties can return None, because ManimConfig.__init__() initializes the internal ._d dictionary whose values are all None in the beginning, before digesting any arguments. This makes the config even harder to use with type checking. Currently, the Manim configuration is very convoluted and I don't think that MyPy errors can stop being ignored right now in mypy.ini, but there have been multiple attempts to refactor and clean the config, so I would wait for those PRs. In the meantime, I would say that simply returning Path is fine.

Suggested change

def media_dir(self) -> str | Path:

def media_dir(self) -> Path:

chopan050 · 2024-12-23T04:23:01Z

manim/renderer/cairo_renderer.py

@@ -160,7 +161,7 @@ def render(self, scene, time, moving_mobjects):
        self.update_frame(scene, moving_mobjects)
        self.add_frame(self.get_frame())

-    def get_frame(self):
+    def get_frame(self) -> npt.NDArray:


There's a type alias in manim.utils.typing for this specific case:

Suggested change

def get_frame(self) -> npt.NDArray:

def get_frame(self) -> RGBPixelArray:

chopan050 · 2024-12-23T04:24:43Z

manim/scene/scene.py

@@ -486,7 +486,7 @@ def add(self, *mobjects: Mobject):
                self.moving_mobjects += mobjects
        return self

-    def add_mobjects_from_animations(self, animations):
+    def add_mobjects_from_animations(self, animations: list) -> None:


When a parameter or return type is a list, you can (and should) specify its contents explicitly. In this case:

Suggested change

def add_mobjects_from_animations(self, animations: list) -> None:

def add_mobjects_from_animations(self, animations: list[Animation]) -> None:

chopan050 · 2024-12-23T04:29:59Z

manim/scene/scene_file_writer.py

-    def __init__(self, renderer, scene_name, **kwargs):
+    def __init__(
+        self,
+        renderer: Any,


Suggested change

renderer: Any,

renderer: CairoRenderer | OpenGLRenderer,

In this way, the type checker can detect the .num_plays attribute which is referenced twice in this file.

chopan050 · 2024-12-23T06:32:23Z

manim/utils/module_ops.py

+def prompt_user_for_choice(scene_classes: list[Any]) -> list[Any]:
    num_to_class = {}


Suggested change

def prompt_user_for_choice(scene_classes: list[Any]) -> list[Any]:

num_to_class = {}

def prompt_user_for_choice(scene_classes: list[type[Scene]]) -> list[type[Scene]]:

num_to_class: dict[int, type[Scene]] = {}

chopan050 · 2024-12-23T06:37:32Z

manim/utils/module_ops.py

@@ -125,8 +132,8 @@ def prompt_user_for_choice(scene_classes):


 def scene_classes_from_file(


This one could be overloaded like this:

@overload def scene_classes_from_file( file_path: Path, require_single_scene: bool, full_list: Literal[True] ) -> list[type[Scene]]: ... @overload def scene_classes_from_file( file_path: Path, require_single_scene: Literal[True], full_list: Literal[False] = False ) -> type[Scene]: ... @overload def scene_classes_from_file( file_path: Path, require_single_scene: Literal[False] = False, full_list: Literal[False] = False ) -> list[type[Scene]]: ... def scene_classes_from_file( file_path: Path, require_single_scene: bool = False, full_list: bool = False ) -> type[Scene] | list[type[Scene]]: # the rest

chopan050 · 2024-12-23T06:37:47Z

manim/utils/opengl.py

@@ -22,17 +27,17 @@
 ]


-def matrix_to_shader_input(matrix):
+def matrix_to_shader_input(matrix: npt.NDArray) -> tuple:


All these functions use 4x4 matrices. Maybe we could define a Matrix4x4: TypeAlias = MatrixMN in manim.typing. I do suggest defining FlattenedMatrix4x4, because many functions here use it.

Suggested change

def matrix_to_shader_input(matrix: npt.NDArray) -> tuple:

FlattenedMatrix4x4: TypeAlias = tuple[

float,

float,

float,

float,

float,

float,

float,

float,

float,

float,

float,

float,

float,

float,

float,

float,

]

def matrix_to_shader_input(matrix: MatrixMN) -> FlattenedMatrix4x4:

chopan050 · 2024-12-23T06:38:16Z

manim/utils/opengl.py

+    near: float = 1,
+    far: float = depth + 1,
+    format_: bool = True,
+) -> npt.NDArray | tuple:


Suggested change

) -> npt.NDArray | tuple:

) -> MatrixMN | FlattenedMatrix4x4:

chopan050 · 2024-12-23T06:38:32Z

manim/utils/opengl.py

+    near: float = 2,
+    far: float = 50,
+    format_: bool = True,
+) -> npt.NDArray | tuple:


Suggested change

) -> npt.NDArray | tuple:

) -> MatrixMN | FlattenedMatrix4x4:

chopan050

Part 2 of 3:

chopan050 · 2024-12-23T18:14:45Z

manim/utils/opengl.py

        return matrix_to_shader_input(projection_matrix)
    else:
        return projection_matrix


-def translation_matrix(x=0, y=0, z=0):
+def translation_matrix(x: float = 0, y: float = 0, z: float = 0) -> npt.NDArray:


Suggested change

def translation_matrix(x: float = 0, y: float = 0, z: float = 0) -> npt.NDArray:

def translation_matrix(x: float = 0, y: float = 0, z: float = 0) -> MatrixMN:

chopan050 · 2024-12-23T18:17:28Z

manim/utils/opengl.py

@@ -83,7 +92,7 @@ def translation_matrix(x=0, y=0, z=0):
    )


If x, y and z all happen to be integers, then this array would be interpreted as an matrix of integers rather than floats.
I suggest adding the float dtype explicitly.

Suggested change

)

dtype=ManimFloat,

)

chopan050 · 2024-12-23T18:17:43Z

manim/utils/opengl.py

@@ -83,7 +92,7 @@ def translation_matrix(x=0, y=0, z=0):
    )


-def x_rotation_matrix(x=0):
+def x_rotation_matrix(x: float = 0) -> npt.NDArray:


Suggested change

def x_rotation_matrix(x: float = 0) -> npt.NDArray:

def x_rotation_matrix(x: float = 0) -> MatrixMN:

chopan050 · 2024-12-23T18:17:55Z

manim/utils/opengl.py

@@ -94,7 +103,7 @@ def x_rotation_matrix(x=0):
    )


-def y_rotation_matrix(y=0):
+def y_rotation_matrix(y: float = 0) -> npt.NDArray:


Suggested change

def y_rotation_matrix(y: float = 0) -> npt.NDArray:

def y_rotation_matrix(y: float = 0) -> MatrixMN:

chopan050 · 2024-12-23T18:18:03Z

manim/utils/opengl.py

@@ -105,7 +114,7 @@ def y_rotation_matrix(y=0):
    )


-def z_rotation_matrix(z=0):
+def z_rotation_matrix(z: float = 0) -> npt.NDArray:


Suggested change

def z_rotation_matrix(z: float = 0) -> npt.NDArray:

def z_rotation_matrix(z: float = 0) -> MatrixMN:

chopan050 · 2024-12-24T03:53:52Z

manim/utils/space_ops.py

@@ -637,7 +645,8 @@ def shoelace(x_y: np.ndarray) -> float:
    """
    x = x_y[:, 0]
    y = x_y[:, 1]
-    return np.trapz(y, x)
+    val: float = np.trapz(y, x)
+    return val


 def shoelace_direction(x_y: np.ndarray) -> str:


Suggested change

def shoelace_direction(x_y: np.ndarray) -> str:

def shoelace_direction(x_y: Point2D_Array) -> str:

chopan050 · 2024-12-24T03:54:17Z

manim/utils/space_ops.py

@@ -758,7 +767,7 @@ def earclip_triangulation(verts: np.ndarray, ring_ends: list) -> list:
            raise Exception("Could not find a ring to attach")

    # Setup linked list
-    after = []
+    after: list = []


Suggested change

after: list = []

after: list[int] = []

chopan050 · 2024-12-24T03:56:12Z

manim/utils/space_ops.py

@@ -829,7 +838,7 @@ def spherical_to_cartesian(spherical: Sequence[float]) -> np.ndarray:

 def perpendicular_bisector(
    line: Sequence[np.ndarray],


Suggested change

line: Sequence[np.ndarray],

line: Point3DLike_Array,

chopan050 · 2024-12-24T03:56:15Z

manim/utils/space_ops.py

@@ -829,7 +838,7 @@ def spherical_to_cartesian(spherical: Sequence[float]) -> np.ndarray:

 def perpendicular_bisector(
    line: Sequence[np.ndarray],
-    norm_vector=OUT,
+    norm_vector: Vector3D = OUT,
 ) -> Sequence[np.ndarray]:


Suggested change

) -> Sequence[np.ndarray]:

) -> list[Point3D]:

chopan050 · 2024-12-24T03:58:30Z

manim/utils/testing/_frames_testers.py

        self._file_path = file_path
        self._show_diff = show_diff
        self._frames: np.ndarray
        self._number_frames: int = 0
        self._frames_compared = 0

    @contextlib.contextmanager
-    def testing(self):
+    def testing(self) -> Generator[Any, Any, Any]:


Wouldn't this be

Suggested change

def testing(self) -> Generator[Any, Any, Any]:

def testing(self) -> Generator[None, None, None]:

since it yields nothing?

github-advanced-security bot found potential problems Nov 4, 2024

View reviewed changes

henrikmidtiby mentioned this pull request Nov 4, 2024

Type annotations for manim/scene/*.py #3981

Open

3 tasks

github-advanced-security bot found potential problems Nov 5, 2024

View reviewed changes

github-advanced-security bot found potential problems Nov 6, 2024

View reviewed changes

manim/utils/family_ops.py Dismissed Show dismissed Hide dismissed

github-advanced-security bot found potential problems Nov 12, 2024

View reviewed changes

henrikmidtiby added 23 commits November 12, 2024 16:53

Handled mypy issues in utils/bezier.py

6e0d440

Disable mypy errors in manim.utils.*

8c887b5

Fix mypy errors in utils/unit.py

09b10c7

Handle mypy errors in utils/debug.py

5868534

Fix mypy issues in utils.color.*

57d4971

Avoid circular import.

f764767

Handle mypy errors in utils.simple_functions.*

1021c47

Handle my errors in utils.testing.*

0002711

Avoid circular import.

df742bc

Handle mypy errors in utils/family_ops.py

ce85521

Handle mypy errors in utils/parameter_parsing.py

21c1a44

Handle some of the mypy errors in utils.docbuild.*

e155bae

Handle mypy errors for utils/config_ops.py

2f5480d

Handle mypy errors from utils/commands.py

97227b2

Handle mypy errors in utils/tex_templates.py

3a2eaee

Handle mypy errors in utils/space_ops.py

d21525d

Fixed most type errors in utils/rate_functions.py

1b1e417

Handle type errors in utils/paths.py

7bb63bd

Handled type errors in utils/deprecation.py

a26da20

Handle type errors in utils/tex_file_writing.py

3b2e31d

Handle type error in utils/sounds.py

b1e815e

Handle type errors in utils/opengl.py

f086fec

Handle type errors in utils/module_ops.py

d51f929

henrikmidtiby added 3 commits November 12, 2024 16:53

Handle type errors in utils/caching.py, utils/familily.py and utils/i…

b0bd14e

…mages.py

Handle type errors in utils/file_ops.py

2f151d7

Handle type errors in utils/ipython_magic.py

224db6d

henrikmidtiby force-pushed the type_annotation_utils branch from cf40c7c to 224db6d Compare November 12, 2024 15:53

henrikmidtiby mentioned this pull request Nov 14, 2024

Typing progress tracker #3375

Open

22 tasks

chopan050 requested changes Dec 23, 2024

View reviewed changes

chopan050 requested changes Dec 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding type annotations to manim.utils.* #3999

Adding type annotations to manim.utils.* #3999

henrikmidtiby commented Nov 4, 2024

henrikmidtiby commented Nov 5, 2024

JasonGrace2282 commented Nov 5, 2024

chopan050 left a comment

chopan050 Dec 23, 2024

chopan050 Dec 23, 2024

chopan050 Dec 23, 2024

chopan050 Dec 23, 2024

chopan050 Dec 23, 2024

chopan050 Dec 23, 2024

chopan050 Dec 23, 2024

chopan050 Dec 23, 2024 •

edited

Loading

chopan050 Dec 23, 2024 •

edited

Loading

chopan050 Dec 23, 2024 •

edited

Loading

chopan050 left a comment

chopan050 Dec 23, 2024

chopan050 Dec 23, 2024

chopan050 Dec 23, 2024

chopan050 Dec 23, 2024

chopan050 Dec 23, 2024

chopan050 Dec 24, 2024

chopan050 Dec 24, 2024

chopan050 Dec 24, 2024

chopan050 Dec 24, 2024

chopan050 Dec 24, 2024



		@overload
		def clip(a: str, min_a: str, max_a: str) -> str: ...

	def digest_args(self, args: argparse.Namespace \| list[str]) -> Self:
	def digest_args(self, args: argparse.Namespace) -> Self:

	def media_dir(self) -> str \| Path:
	def media_dir(self) -> Path:

	def get_frame(self) -> npt.NDArray:
	def get_frame(self) -> RGBPixelArray:

	def add_mobjects_from_animations(self, animations: list) -> None:
	def add_mobjects_from_animations(self, animations: list[Animation]) -> None:

		def prompt_user_for_choice(scene_classes: list[Any]) -> list[Any]:
		num_to_class = {}

		@@ -125,8 +132,8 @@ def prompt_user_for_choice(scene_classes):


		def scene_classes_from_file(

-def matrix_to_shader_input(matrix: npt.NDArray) -> tuple:
+FlattenedMatrix4x4: TypeAlias = tuple[
+    float,
+    float,
+    float,
+    float,
+    float,
+    float,
+    float,
+    float,
+    float,
+    float,
+    float,
+    float,
+    float,
+    float,
+    float,
+    float,
+]
+def matrix_to_shader_input(matrix: MatrixMN) -> FlattenedMatrix4x4:

	) -> npt.NDArray \| tuple:
	) -> MatrixMN \| FlattenedMatrix4x4:

	def translation_matrix(x: float = 0, y: float = 0, z: float = 0) -> npt.NDArray:
	def translation_matrix(x: float = 0, y: float = 0, z: float = 0) -> MatrixMN:

	def x_rotation_matrix(x: float = 0) -> npt.NDArray:
	def x_rotation_matrix(x: float = 0) -> MatrixMN:

	def y_rotation_matrix(y: float = 0) -> npt.NDArray:
	def y_rotation_matrix(y: float = 0) -> MatrixMN:

	def z_rotation_matrix(z: float = 0) -> npt.NDArray:
	def z_rotation_matrix(z: float = 0) -> MatrixMN:

	def shoelace_direction(x_y: np.ndarray) -> str:
	def shoelace_direction(x_y: Point2D_Array) -> str:

		@@ -829,7 +838,7 @@ def spherical_to_cartesian(spherical: Sequence[float]) -> np.ndarray:

		def perpendicular_bisector(
		line: Sequence[np.ndarray],

	def testing(self) -> Generator[Any, Any, Any]:
	def testing(self) -> Generator[None, None, None]:

Adding type annotations to manim.utils.* #3999

Are you sure you want to change the base?

Adding type annotations to manim.utils.* #3999

Conversation

henrikmidtiby commented Nov 4, 2024

Overview: What does this pull request change?

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

Further Information and Comments

Reviewer Checklist

henrikmidtiby commented Nov 5, 2024

JasonGrace2282 commented Nov 5, 2024

chopan050 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chopan050 Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

chopan050 Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

chopan050 Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

chopan050 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chopan050 Dec 23, 2024 •

edited

Loading

chopan050 Dec 23, 2024 •

edited

Loading

chopan050 Dec 23, 2024 •

edited

Loading