Add type annotations to mobject_update_utils.py
#4382
Conversation
There was a problem hiding this comment.
Thanks!
This is very related to an old open PR of mine: #3976. Although that PR is mainly focused on improving docstrings and adding examples, it also seeks to refine the typehints and use typevars in order to get better autocompletion (for example, if you use brace = always_redraw(lambda: Brace(...)), my idea is that brace gets autocompletion for all of the attributes and methods of Brace, rather than only the Mobject ones).
Both PRs overlap quite a lot. However, personally, I would like to merge this one first, as I've been noticing that my PR lacks some extra polishing: I didn't stop ignoring MyPy errors for that file and, when I started raising them, many errors appeared that don't have an easy solution. You might want to take some inspiration from there if you wish so, but beware of new, sneaky MyPy errors appearing if you do so. Some of that stuff is probably overengineered and I have to take a new look at it when I have time. In the meantime, I'll mark my PR as a draft.
| @@ -26,6 +26,7 @@ | |||
|
|
|||
| if TYPE_CHECKING: | |||
| from manim.animation.animation import Animation | |||
| from manim.typing import Vector3DLike | |||
|
|
|||
|
|
|||
| def assert_is_mobject_method(method: Callable) -> None: | |||
There was a problem hiding this comment.
Ideally, all the errors you describe below (method not having attributes __self__ and __func__) could be fixed by typing the method parameter as a MethodType instead, which you have to import from the types standard library.
However, if you try to pass a method directly to these functions after typing method as MethodType, MyPy raises an error. There's an issue about this on the MyPy GitHub: python/mypy#14235
In the meantime, I would say it's still fine to type them as MethodType to prevent these errors you're having. We already have some parameters and returned values typed as MethodType anyways.
EDIT: also see my implementation of MobjectMethod in PR #3976 if you're interested. The point of it is getting better autocompletion for Mobjects, but it still suffers from the issue described above (and I forgot to include the __self__ attribute).
| def assert_is_mobject_method(method: Callable) -> None: | |
| def assert_is_mobject_method(method: MethodType) -> None: |
| @@ -34,33 +35,39 @@ def assert_is_mobject_method(method: Callable) -> None: | |||
| assert isinstance(mobject, (Mobject, OpenGLMobject)) | |||
|
|
|||
|
|
|||
| def always(method: Callable, *args, **kwargs) -> Mobject: | |||
| def always(method: Callable, *args: Any, **kwargs: Any) -> Mobject | OpenGLMobject: | |||
There was a problem hiding this comment.
| def always(method: Callable, *args: Any, **kwargs: Any) -> Mobject | OpenGLMobject: | |
| def always(method: MethodType, *args: Any, **kwargs: Any) -> Mobject | OpenGLMobject: |
| func = method.__func__ | ||
| mobject.add_updater(lambda m: func(m, *args, **kwargs)) | ||
| return mobject | ||
|
|
||
|
|
||
| def f_always(method: Callable[[Mobject], None], *arg_generators, **kwargs) -> Mobject: | ||
| def f_always( | ||
| method: Callable[[Mobject], None], *arg_generators: Any, **kwargs: Any |
There was a problem hiding this comment.
| method: Callable[[Mobject], None], *arg_generators: Any, **kwargs: Any | |
| method: MethodType, *arg_generators: Any, **kwargs: Any |
| args = [arg_generator() for arg_generator in arg_generators] | ||
| func(mob, *args, **kwargs) | ||
|
|
||
| mobject.add_updater(updater) | ||
| return mobject | ||
|
|
||
|
|
||
| def always_redraw(func: Callable[[], Mobject]) -> Mobject: | ||
| def always_redraw(func: Callable[[], Mobject]) -> Mobject | OpenGLMobject: |
There was a problem hiding this comment.
If the return value is any of these two classes, the func should be able to return them:
| def always_redraw(func: Callable[[], Mobject]) -> Mobject | OpenGLMobject: | |
| def always_redraw( | |
| func: Callable[[], Mobject | OpenGLMobject] | |
| ) -> Mobject | OpenGLMobject: |
Overview: What does this pull request change?
Reviewer Checklist