Type annotations for manim/scene/*.py

henrikmidtiby commented 1 month ago

Overview: What does this pull request change?

The pull request add type annotations to the files manim/scene/*.py and some of its dependencies. The request is still a work in progress, as mypy currently lists 90 errors. With the pull request I hope to get some feedback and advise on how to proceed with the work.

The pull request is a continuation of the work in https://github.com/ManimCommunity/manim/pull/3961

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

Type annotations help identify errors during development.

Further Information and Comments

I would suggest to make some changes to certain default values. https://github.com/henrikmidtiby/manim/blob/6c137ee0f9f7b414bd5c1e0b6657eeaab005e088/manim/scene/scene.py#L128-L131 https://github.com/henrikmidtiby/manim/blob/6c137ee0f9f7b414bd5c1e0b6657eeaab005e088/manim/scene/scene.py#L119-L122 https://github.com/henrikmidtiby/manim/blob/6c137ee0f9f7b414bd5c1e0b6657eeaab005e088/manim/scene/scene.py#L119-L122

A change related to the background of a new Camera instance. https://github.com/henrikmidtiby/manim/blob/6c137ee0f9f7b414bd5c1e0b6657eeaab005e088/manim/scene/vector_space_scene.py#L133-L135

It appears as the property target_text is not really used for something. https://github.com/henrikmidtiby/manim/blob/6c137ee0f9f7b414bd5c1e0b6657eeaab005e088/manim/scene/vector_space_scene.py#L1117-L1122

In some cases I have chosen to insert assertions about the type of certain data. See e.g. here. Is this a suitable approach? https://github.com/henrikmidtiby/manim/blob/6c137ee0f9f7b414bd5c1e0b6657eeaab005e088/manim/scene/scene.py#L1333

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

henrikmidtiby commented 3 weeks ago

@JasonGrace2282 I need some help to proceed with this work.

At the current state of the code, there are 75 errors from mypy when the errors from mypy-manim.scene is no longer ignored. The 75 errors are listed below. I would love to get rid of the issues, but I think that it requires more indepth knowledge of the codebase and the intended code style to choose the proper solutions.

$ mypy
manim/scene/zoomed_scene.py:180: error: Item "OpenGLCamera" of "Camera | OpenGLCamera" has no attribute "frame_height"  [union-attr]
manim/scene/zoomed_scene.py:181: error: Item "OpenGLCamera" of "Camera | OpenGLCamera" has no attribute "frame_width"  [union-attr]
manim/scene/vector_space_scene.py:132: error: Missing positional argument "scene" in call to "update_frame" of "CairoRenderer"  [call-arg]
manim/scene/vector_space_scene.py:132: error: Missing positional argument "scene" in call to "update_frame"  [call-arg]
manim/scene/vector_space_scene.py:157: error: "VectorScene" has no attribute "plane"  [attr-defined]
manim/scene/vector_space_scene.py:158: error: "VectorScene" has no attribute "plane"  [attr-defined]
manim/scene/vector_space_scene.py:548: error: Argument 1 to "VMobject" has incompatible type "*Generator[Dot, None, None]"; expected "ManimColor | int | str | tuple[int, int, int] | tuple[float, float, float] | <6 more items> | None"  [arg-type]
manim/scene/vector_space_scene.py:548: error: Argument 1 to "VMobject" has incompatible type "*Generator[Dot, None, None]"; expected "float"  [arg-type]
manim/scene/vector_space_scene.py:548: error: Argument 1 to "VMobject" has incompatible type "*Generator[Dot, None, None]"; expected "LineJointType | None"  [arg-type]
manim/scene/vector_space_scene.py:548: error: Argument 1 to "VMobject" has incompatible type "*Generator[Dot, None, None]"; expected "ndarray[Any, dtype[floating[_64Bit]]]"  [arg-type]
manim/scene/vector_space_scene.py:548: error: Argument 1 to "VMobject" has incompatible type "*Generator[Dot, None, None]"; expected "bool"  [arg-type]
manim/scene/vector_space_scene.py:548: error: Argument 1 to "VMobject" has incompatible type "*Generator[Dot, None, None]"; expected "Image | str | None"  [arg-type]
manim/scene/vector_space_scene.py:548: error: Argument 1 to "VMobject" has incompatible type "*Generator[Dot, None, None]"; expected "int"  [arg-type]
manim/scene/vector_space_scene.py:548: error: Argument 1 to "VMobject" has incompatible type "*Generator[Dot, None, None]"; expected "CapStyleType"  [arg-type]
manim/scene/vector_space_scene.py:677: error: Argument 1 to "NumberPlane" has incompatible type "**dict[str, object]"; expected "Sequence[float] | None"  [arg-type]
manim/scene/vector_space_scene.py:677: error: Argument 1 to "NumberPlane" has incompatible type "**dict[str, object]"; expected "float | None"  [arg-type]
manim/scene/vector_space_scene.py:677: error: Argument 1 to "NumberPlane" has incompatible type "**dict[str, object]"; expected "dict[str, Any] | None"  [arg-type]
manim/scene/vector_space_scene.py:677: note: "Dict" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance
manim/scene/vector_space_scene.py:677: note: Consider using "Mapping" instead, which is covariant in the value type
manim/scene/vector_space_scene.py:677: error: Argument 1 to "NumberPlane" has incompatible type "**dict[str, object]"; expected "int"  [arg-type]
manim/scene/vector_space_scene.py:677: error: Argument 1 to "NumberPlane" has incompatible type "**dict[str, object]"; expected "bool"  [arg-type]
manim/scene/vector_space_scene.py:677: error: Argument 1 to "NumberPlane" has incompatible type "**dict[str, object]"; expected "dict[str, Any]"  [arg-type]
manim/scene/vector_space_scene.py:684: error: Argument 1 to "NumberPlane" has incompatible type "**dict[str, object]"; expected "Sequence[float] | None"  [arg-type]
manim/scene/vector_space_scene.py:684: error: Argument 1 to "NumberPlane" has incompatible type "**dict[str, object]"; expected "float | None"  [arg-type]
manim/scene/vector_space_scene.py:684: error: Argument 1 to "NumberPlane" has incompatible type "**dict[str, object]"; expected "dict[str, Any] | None"  [arg-type]
manim/scene/vector_space_scene.py:684: note: "Dict" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance
manim/scene/vector_space_scene.py:684: note: Consider using "Mapping" instead, which is covariant in the value type
manim/scene/vector_space_scene.py:684: error: Argument 1 to "NumberPlane" has incompatible type "**dict[str, object]"; expected "int"  [arg-type]
manim/scene/vector_space_scene.py:684: error: Argument 1 to "NumberPlane" has incompatible type "**dict[str, object]"; expected "bool"  [arg-type]
manim/scene/vector_space_scene.py:684: error: Argument 1 to "NumberPlane" has incompatible type "**dict[str, object]"; expected "dict[str, Any]"  [arg-type]
manim/scene/vector_space_scene.py:729: error: Signature of "add_foreground_mobject" incompatible with supertype "Scene"  [override]
manim/scene/vector_space_scene.py:729: note:      Superclass:
manim/scene/vector_space_scene.py:729: note:          def add_foreground_mobject(self, mobject: Mobject) -> Scene
manim/scene/vector_space_scene.py:729: note:      Subclass:
manim/scene/vector_space_scene.py:729: note:          def add_foreground_mobject(self, *mobjects: Mobject) -> None
manim/scene/vector_space_scene.py:940: error: "MathTex" has no attribute "target_text"  [attr-defined]
manim/scene/vector_space_scene.py:942: error: "MathTex" has no attribute "target_text"  [attr-defined]
manim/scene/vector_space_scene.py:945: error: "MathTex" has no attribute "vector"  [attr-defined]
manim/scene/vector_space_scene.py:946: error: "MathTex" has no attribute "kwargs"  [attr-defined]
manim/scene/vector_space_scene.py:947: error: "Never" has no attribute "__iter__" (not iterable)  [attr-defined]
manim/scene/vector_space_scene.py:948: error: "Never" has no attribute "pop"  [attr-defined]
manim/scene/vector_space_scene.py:1119: error: Incompatible types in assignment (expression has type "MethodType", variable has type "MathTex | str")  [assignment]
manim/scene/vector_space_scene.py:1120: error: Argument after ** must be a mapping, not "MethodType"  [arg-type]
manim/scene/vector_space_scene.py:1121: error: "MethodType" has no attribute "target"  [attr-defined]
manim/scene/three_d_scene.py:138: error: Incompatible types in assignment (expression has type "ThreeDCamera", variable has type "OpenGLCamera")  [assignment]
manim/scene/three_d_scene.py:167: error: "ThreeDCamera" has no attribute "clear_updaters"  [attr-defined]
manim/scene/three_d_scene.py:282: error: "ThreeDCamera" has no attribute "copy"  [attr-defined]
manim/scene/three_d_scene.py:297: error: "ThreeDCamera" has no attribute "height"  [attr-defined]
manim/scene/three_d_scene.py:299: error: "object" object is not iterable  [misc]
manim/scene/three_d_scene.py:307: error: Cannot determine type of "method"  [has-type]
manim/scene/three_d_scene.py:315: error: Argument "mobject" to "Transform" has incompatible type "ThreeDCamera"; expected "Mobject | None"  [arg-type]
manim/scene/three_d_scene.py:449: error: "ThreeDScene" has no attribute "default_camera_orientation_kwargs"; maybe "default_angled_camera_orientation_kwargs" or "set_camera_orientation"?  [attr-defined]
manim/scene/three_d_scene.py:505: error: "CairoRenderer" has no attribute "camera_config"  [attr-defined]
manim/scene/scene_file_writer.py:45: error: Function is missing a type annotation  [no-untyped-def]
manim/scene/scene_file_writer.py:62: error: Function is missing a return type annotation  [no-untyped-def]
manim/scene/scene_file_writer.py:70: error: "Stream" has no attribute "encode"  [attr-defined]
manim/scene/scene_file_writer.py:73: error: "Stream" has no attribute "encode"  [attr-defined]
manim/scene/scene_file_writer.py:372: error: Argument 2 to "convert_audio" has incompatible type "_TemporaryFileWrapper[bytes]"; expected "Path"  [arg-type]
manim/scene/scene_file_writer.py:433: error: Argument 1 to "from_ndarray" of "VideoFrame" has incompatible type "ndarray[Any, dtype[signedinteger[_64Bit]]]"; expected "ndarray[Any, dtype[unsignedinteger[_8Bit]]] | ndarray[Any, dtype[unsignedinteger[_16Bit]]] | ndarray[Any, dtype[floating[_32Bit]]]"  [arg-type]
manim/scene/scene_file_writer.py:434: error: Cannot determine type of "video_stream"  [has-type]
manim/scene/scene_file_writer.py:435: error: Cannot determine type of "video_container"  [has-type]
manim/scene/scene_file_writer.py:453: error: Item "ndarray[Any, Any]" of "ndarray[Any, Any] | OpenGLRenderer" has no attribute "get_frame"  [union-attr]
manim/scene/scene_file_writer.py:463: error: Item "ndarray[Any, Any]" of "ndarray[Any, Any] | OpenGLRenderer" has no attribute "get_image"  [union-attr]
manim/scene/scene_file_writer.py:465: error: Argument 1 to "fromarray" has incompatible type "ndarray[Any, Any] | OpenGLRenderer"; expected "SupportsArrayInterface"  [arg-type]
manim/scene/scene_file_writer.py:536: error: Call to untyped function "to_av_frame_rate" in typed context  [no-untyped-call]
manim/scene/scene_file_writer.py:664: error: Call to untyped function "to_av_frame_rate" in typed context  [no-untyped-call]
manim/scene/scene_file_writer.py:690: error: Incompatible types in assignment (expression has type "VideoFrame | AudioFrame", variable has type "VideoFrame")  [assignment]
manim/scene/scene.py:156: error: Argument "camera_class" to "CairoRenderer" has incompatible type "type[Camera]"; expected "Camera | None"  [arg-type]
manim/scene/scene.py:183: error: Argument 2 to "deepcopy" has incompatible type "list[Any]"; expected "dict[int, Any] | None"  [arg-type]
manim/scene/scene.py:229: error: Missing return statement  [return]
manim/scene/scene.py:246: error: Item "CairoRenderer" of "CairoRenderer | OpenGLRenderer" has no attribute "clear_screen"  [union-attr]
manim/scene/scene.py:459: error: Name "OpenGLMobject" is not defined  [name-defined]
manim/scene/scene.py:485: error: Argument 1 to "remove" of "Scene" has incompatible type "*list[Object3D]"; expected "Mobject"  [arg-type]
manim/scene/scene.py:512: error: Argument 1 to "__iadd__" of "list" has incompatible type "Sequence[OpenGLMobject]"; expected "Iterable[Mobject]"  [arg-type]
manim/scene/scene.py:1325: error: Item "OpenGLRenderer" of "CairoRenderer | OpenGLRenderer" has no attribute "static_image"  [union-attr]
manim/scene/scene.py:1386: error: Item "None" of "FrameType | None" has no attribute "f_back"  [union-attr]
manim/scene/scene.py:1386: error: Item "None" of "FrameType | Any | None" has no attribute "f_locals"  [union-attr]
manim/scene/scene.py:1498: error: Item "None" of "PromptSession[Any] | None" has no attribute "app"  [union-attr]
manim/scene/scene.py:1540: error: Item "None" of "FrameType | None" has no attribute "f_locals"  [union-attr]
manim/scene/scene.py:1669: error: Item "CairoRenderer" of "CairoRenderer | OpenGLRenderer" has no attribute "pressed_keys"  [union-attr]
manim/scene/scene.py:1681: error: Argument "about_point" to "scale" of "OpenGLMobject" has incompatible type "ndarray[Any, dtype[floating[_64Bit]]]"; expected "Sequence[float] | None"  [arg-type]
manim/scene/scene.py:1699: error: Too few arguments  [call-arg]
manim/scene/moving_camera_scene.py:138: error: Item "OpenGLCamera" of "Any | OpenGLCamera" has no attribute "get_mobjects_indicating_movement"  [union-attr]
Found 75 errors in 6 files (checked 163 source files)

JasonGrace2282 commented 3 weeks ago

Firstly, I recommend that you resolve conflicts with the master branch :) Your issues with OpenGL* not having attributes can probably be type ignored (you'd have to check if they're in the appropriate if config.render block). For the ones about variance, check if you need the mutability. If you don't, Mapping might be a better fit over Dict. Otherwise, I suspect you might need to add a lot of assert statements to get mypy to figure out the right type (see type narrowing)?

henrikmidtiby commented 3 weeks ago

@JasonGrace2282 Thanks for the input. At the moment I can't figure out where to begin with the current state of this PR, so I started to work on a smaller PR here: https://github.com/ManimCommunity/manim/pull/3999 I expect to get back to this PR later.

ManimCommunity / manim