Add 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

manim/utils/color/core.py

manim/scene/scene_file_writer.py

manim/utils/simple_functions.py

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:


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.

manim/_config/utils.py

manim/renderer/cairo_renderer.py

manim/scene/scene.py

manim/scene/scene_file_writer.py

manim/utils/module_ops.py

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

When I try the changes that you suggest in module_ops.py, I get the errors shown below.
I will leave it for now and hope to get some help in resolving it.

manim/utils/module_ops.py:95: error: "Scene" has no attribute "__name__" [attr-defined] manim/utils/module_ops.py:104: error: "Scene" has no attribute "__name__" [attr-defined] manim/utils/module_ops.py:113: error: "Scene" has no attribute "__name__" [attr-defined] manim/utils/module_ops.py:115: error: Incompatible types in assignment (expression has type "Scene", target has type "type[Scene]") [assignment] manim/utils/module_ops.py:121: error: List comprehension has incompatible type List[type[Scene]]; expected List[Scene] [misc] manim/utils/module_ops.py:124: error: "Scene" has no attribute "__name__" [attr-defined] manim/utils/module_ops.py:168: error: Incompatible return value type (got "Scene", expected "type[Scene] | list[type[Scene]]") [return-value] manim/utils/module_ops.py:169: error: Incompatible return value type (got "list[Scene]", expected "type[Scene] | list[type[Scene]]") [return-value] Found 8 errors in 1 file (checked 1 source file)

When I tried myself, everything was OK.
Your errors suggest that you might have used Scene instead of type[Scene].

manim/utils/opengl.py

chopan050

Part 2 of 3:

manim/utils/opengl.py

manim/utils/space_ops.py

manim/utils/testing/_frames_testers.py

chopan050

Part 3 of 3:

manim/utils/testing/_frames_testers.py

manim/utils/testing/_show_diff.py

manim/utils/testing/_test_class_makers.py

manim/utils/tex_file_writing.py

mypy.ini

manim/utils/testing/frames_comparison.py

# Conflicts: # manim/utils/bezier.py

# Conflicts: # manim/utils/docbuild/module_parsing.py

manim/scene/scene_file_writer.py

manim/utils/caching.py

manim/utils/deprecation.py

manim/utils/simple_functions.py



-def clip(a, min_a, max_a):
+class Comparable(Protocol):
+    def __lt__(self, other: Any) -> bool: ...


manim/utils/simple_functions.py

+class Comparable(Protocol):
+    def __lt__(self, other: Any) -> bool: ...
+
+    def __gt__(self, other: Any) -> bool: ...


henrikmidtiby · 2025-01-01T19:21:13Z

@chopan050, thanks for all the comments. I have addressed most of them in the code, as well as rebased the changes on top of the current main branch.
For the future I will go for smaller pull requests, at it is quite difficult to maintain the overview of this one.

manim/utils/hashing.py

manim/utils/module_ops.py

+@overload
+def scene_classes_from_file(
+    file_path: Path, require_single_scene: bool, full_list: Literal[True]
+) -> list[type[Scene]]: ...


manim/utils/module_ops.py

+    file_path: Path,
+    require_single_scene: Literal[True],
+    full_list: Literal[False] = False,
+) -> type[Scene]: ...


manim/utils/module_ops.py

+    file_path: Path,
+    require_single_scene: Literal[False] = False,
+    full_list: Literal[False] = False,
+) -> list[type[Scene]]: ...


manim/utils/rate_functions.py

manim/utils/deprecation.py

    since: str | None = None,
    until: str | None = None,
    replacement: str | None = None,
    message: str | None = "",
-) -> Callable:
+) -> Callable[..., T]: ...


manim/utils/deprecation.py

+    until: str | None = None,
+    replacement: str | None = None,
+    message: str | None = "",
+) -> Callable[[Callable[..., T]], Callable[..., T]]: ...


manim/utils/rate_functions.py

+
+# TODO: rewrite this to use ParamSpec when Python 3.9 is out of life
+class RateFunction(Protocol):
+    def __call__(self, t: float, *args: Any, **kwargs: Any) -> float: ...


…Py errors

chopan050

LGTM! This has been a huge effort, and we really appreciate it! Thank you so much!

If you don't mind, I pushed some changes in order to finish this PR:

I removed caching, polylabel and qhull from mypy.ini after fixing some MyPy errors there. I couldn't, however, remove hashing because it was way too complex to type. Still, this means that almost all of utils is typed, which is great!
manim.utils.module_ops could use type[Scene], as I mentioned in a comment.
I rewrote that function in manim.utils.rate_functions which used bezier, because MyPy wouldn't stop complaining about that.
I realized that ParamSpec was introduced in Python 3.10 and we're still supporting Python 3.9. Therefore, I had to modify the changes where I suggested using ParamSpec. In the future, when we drop support for Python 3.9, we would apply it.
It turns out that the dir properties in ManimConfig always return str, because they're always converted to str when you attempt to set them. MyPy, however, dislikes that the getter returns a str and the setter accepts a str | Path to convert it to str, so this is something which must be fixed in a subsequent PR. In the meantime, I had to use some # type: ignore comments to silence it.
I fixed some other MyPy errors.

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

manim/scene/scene_file_writer.py Fixed Show fixed Hide fixed

manim/scene/scene_file_writer.py Fixed Show fixed Hide fixed

manim/utils/simple_functions.py Fixed Show fixed Hide fixed

manim/utils/simple_functions.py Fixed Show fixed Hide fixed

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 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

chopan050 requested changes Dec 25, 2024

View reviewed changes

henrikmidtiby added 18 commits December 28, 2024 22:51

Handled mypy issues in utils/bezier.py

45d84fd

# Conflicts: # manim/utils/bezier.py

Disable mypy errors in manim.utils.*

c2a715a

Fix mypy errors in utils/unit.py

40250de

Handle mypy errors in utils/debug.py

910aa3f

Fix mypy issues in utils.color.*

8bf07ca

Avoid circular import.

5d2ce00

Handle mypy errors in utils.simple_functions.*

ec07a4f

Handle my errors in utils.testing.*

bb3ec8a

Avoid circular import.

d6210c3

Handle mypy errors in utils/family_ops.py

21599b1

Handle mypy errors in utils/parameter_parsing.py

4997a37

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

3be6129

# Conflicts: # manim/utils/docbuild/module_parsing.py

Handle mypy errors for utils/config_ops.py

3cabd9e

Handle mypy errors from utils/commands.py

29792df

Handle mypy errors in utils/tex_templates.py

9f0be52

Handle mypy errors in utils/space_ops.py

111775b

Fixed most type errors in utils/rate_functions.py

459092e

Handle type errors in utils/paths.py

be12a0f

henrikmidtiby added 11 commits December 28, 2024 23:45

Addressing comments from chopan50, block 7

2a29354

Addressing comments from chopan50, block 8

a50dd1a

Addressing comments from chopan50, block 9

e055ba1

Addressing comments from chopan50, block 9

d5686d1

Addressing comments from chopan50, block 10

b6b1ff0

Addressing comments from chopan50, block 11

f3ed2b1

Addressing comments from chopan50, block 12

3a4bd5b

Addressing comments from chopan50, block 13

98ff7a8

Addressing comments from chopan50, block 14

44a8645

Addressing comments from chopan50, block 15

e1db9ea

Addressing comments from chopan50, block 16

de80832

henrikmidtiby force-pushed the type_annotation_utils branch from 413adfb to de80832 Compare January 1, 2025 19:06

github-advanced-security bot found potential problems Jan 1, 2025

View reviewed changes

henrikmidtiby added 2 commits January 1, 2025 20:14

Addressing comments from chopan50, block 17

0736393

Addressing comments from chopan50, block 18

711b2d8

Removed some more files from mypy.ini and fix remaining errors

3f0d9ac

github-advanced-security bot found potential problems Jan 2, 2025

View reviewed changes

Fix errors in rate_functions.py and be compatible with Python 3.9

35e6597

github-advanced-security bot found potential problems Jan 2, 2025

View reviewed changes

Remove incorrect use of typing.Concatenate from rate_functions.py

8cce30f

github-advanced-security bot found potential problems Jan 2, 2025

View reviewed changes

manim/utils/rate_functions.py

# TODO: rewrite this to use ParamSpec when Python 3.9 is out of life

class RateFunction(Protocol):

def __call__(self, t: float, *args: Any, **kwargs: Any) -> float: ...

Check notice

Code scanning / CodeQL

Statement has no effect Note

This statement has no effect.

chopan050 added 3 commits January 2, 2025 03:13

ManimConfig dirs have always been strings - Fix manim_directive.py My…

3bb4f6b

…Py errors

Fix StrList error in manim_directive.py

0436034

Fix or ignore errors in manim.utils.docbuild

3c5183c

chopan050 approved these changes Jan 2, 2025

View reviewed changes

chopan050 enabled auto-merge (squash) January 2, 2025 07:01

chopan050 changed the title Adding type annotations to manim.utils.* Add type annotations to manim.utils.* Jan 2, 2025

Reverse points in manim.utils.rate_functions.running_start

6931d36

chopan050 merged commit 17d4a7f into ManimCommunity:main Jan 2, 2025
21 of 22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add type annotations to manim.utils.* #3999

Add 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

henrikmidtiby Jan 1, 2025

chopan050 Jan 2, 2025

chopan050 left a comment

chopan050 left a comment

henrikmidtiby commented Jan 1, 2025

chopan050 left a comment •

edited

Loading

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


		def scene_classes_from_file(

Add type annotations to manim.utils.* #3999

Add 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

chopan050 Dec 23, 2024

Choose a reason for hiding this comment

henrikmidtiby Jan 1, 2025

Choose a reason for hiding this comment

chopan050 Jan 2, 2025

Choose a reason for hiding this comment

chopan050 left a comment

Choose a reason for hiding this comment

chopan050 left a comment

Choose a reason for hiding this comment

henrikmidtiby commented Jan 1, 2025

chopan050 left a comment • edited Loading

Choose a reason for hiding this comment

chopan050 left a comment •

edited

Loading