Added type annotations to mobject/svg/brace.py #4309
Conversation
manim/mobject/svg/brace.py
Outdated
| # TODO: This method is only called from the method change_brace_label, which is | ||
| # not called from any location in the current codebase. |
There was a problem hiding this comment.
Although it's not called anywhere, it seems like an useful method to be exposed to the end user. In that case, the real problem would be that it's not documented at all. This could be a good follow-up PR.
| # TODO: This method is only called from the method change_brace_label, which is | |
| # not called from any location in the current codebase. |
| @@ -238,7 +240,7 @@ def __init__( | |||
| font_size: float = DEFAULT_FONT_SIZE, | |||
There was a problem hiding this comment.
The label_constructor above could be typed as type[SingleStringMathTex | Text] to allow passing both TeX and Cairo-rendered texts. SingleStringMathTex is the parent of MathTex, which is the parent of Tex, which is the parent of BulletedList and Title.
Plus, the brace_direction could be typed as Point3DLike (in the future, Vector3DLike).
There was a problem hiding this comment.
Changing the type of label_constructor raises this error
manim/mobject/svg/brace.py:279: error: Argument 1 to "SingleStringMathTex" has incompatible type "*tuple[str, ...]"; expected "float" [arg-type]
manim/mobject/svg/brace.py:279: error: Argument 1 to "SingleStringMathTex" has incompatible type "*tuple[str, ...]"; expected "bool" [arg-type]
manim/mobject/svg/brace.py:279: error: Argument 1 to "SingleStringMathTex" has incompatible type "*tuple[str, ...]"; expected "float | None" [arg-type]
manim/mobject/svg/brace.py:279: error: Argument 1 to "SingleStringMathTex" has incompatible type "*tuple[str, ...]"; expected "TexTemplate | None" [arg-type]
manim/mobject/svg/brace.py:279: error: Argument 1 to "Text" has incompatible type "*tuple[str, ...]"; expected "float" [arg-type]
manim/mobject/svg/brace.py:279: error: Argument 1 to "Text" has incompatible type "*tuple[str, ...]"; expected "dict[str, str]" [arg-type]
manim/mobject/svg/brace.py:279: error: Argument 1 to "Text" has incompatible type "*tuple[str, ...]"; expected "dict[str, tuple[Any, ...]]" [arg-type]
manim/mobject/svg/brace.py:279: error: Argument 1 to "Text" has incompatible type "*tuple[str, ...]"; expected "tuple[Any, ...]" [arg-type]
manim/mobject/svg/brace.py:279: error: Argument 1 to "Text" has incompatible type "*tuple[str, ...]"; expected "int" [arg-type]
manim/mobject/svg/brace.py:279: error: Argument 1 to "Text" has incompatible type "*tuple[str, ...]"; expected "bool" [arg-type]
for more information, see https://pre-commit.ci
| from manim.utils.color.core import ParsableManimColor | ||
|
|
||
| __all__ = ["Brace", "BraceBetweenPoints", "BraceLabel", "ArcBrace"] | ||
|
|
There was a problem hiding this comment.
Do not change the value of the __all__ variable. This line made it impossible to use the BraceText object.
| def __init__(self, obj, text, label_constructor=Tex, **kwargs): | ||
| def __init__( | ||
| self, obj: Mobject, text: str, label_constructor: type[SingleStringMathTex | Text] = Text, **kwargs: Any | ||
| ): |
There was a problem hiding this comment.
I would argue that the default label_constructor should be Text.
| def change_label(self, *text, **kwargs): | ||
| self.label = self.label_constructor(*text, **kwargs) | ||
|
|
||
| def change_label(self, *text: str, **kwargs: Any) -> Self: |
There was a problem hiding this comment.
I have created this small example to demonstrate the use of the change_label method, but apparently calling the method has no effect on the generated output.
from manim import *
class BraceExample(Scene):
def construct(self):
s1 = Square().move_to(2*LEFT)
self.add(s1)
br1 = BraceText(s1, "Label")
self.add(br1)
s2 = Square().move_to(2*RIGHT)
self.add(s2)
br2 = BraceText(s2, "Label")
# I would expect this line to change the label
# of the br2 brace to "new". But this is not seen
# in the output.
br2.change_label("new")
self.add(br2)
Overview: What does this pull request change?
Type annotations were added to the brace.py file.
Reviewer Checklist