Skip to content

Added type annotations to mobject/svg/brace.py #4309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 34 additions & 18 deletions manim/mobject/svg/brace.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
__all__ = ["Brace", "BraceLabel", "ArcBrace", "BraceText", "BraceBetweenPoints"]

from collections.abc import Sequence
from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, Any

import numpy as np
import svgelements as se
from typing_extensions import Self

from manim._config import config
from manim.mobject.geometry.arc import Arc
Expand All @@ -17,6 +18,7 @@
from manim.mobject.opengl.opengl_compatibility import ConvertToOpenGL
from manim.mobject.text.tex_mobject import MathTex, Tex

from ...animation.animation import Animation
from ...animation.composition import AnimationGroup
from ...animation.fading import FadeIn
from ...animation.growing import GrowFromCenter
Expand All @@ -26,7 +28,7 @@
from ..svg.svg_mobject import VMobjectFromSVGPath

if TYPE_CHECKING:
from manim.typing import Point3DLike, Vector3D
from manim.typing import Point3D, Point3DLike, Vector3D
from manim.utils.color.core import ParsableManimColor

__all__ = ["Brace", "BraceBetweenPoints", "BraceLabel", "ArcBrace"]
Expand Down Expand Up @@ -70,14 +72,14 @@ def construct(self):
def __init__(
self,
mobject: Mobject,
direction: Vector3D | None = DOWN,
direction: Vector3D = DOWN,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might this be typed as Point3DLike in the meantime?

Suggested change
direction: Vector3D = DOWN,
direction: Point3DLike = DOWN,

buff: float = 0.2,
sharpness: float = 2,
stroke_width: float = 0,
fill_opacity: float = 1.0,
background_stroke_width: float = 0,
background_stroke_color: ParsableManimColor = BLACK,
**kwargs,
**kwargs: Any,
):
path_string_template = (
"m0.01216 0c-0.01152 0-0.01216 6.103e-4 -0.01216 0.01311v0.007762c0.06776 "
Expand Down Expand Up @@ -130,7 +132,7 @@ def __init__(
for mob in mobject, self:
mob.rotate(angle, about_point=ORIGIN)

def put_at_tip(self, mob: Mobject, use_next_to: bool = True, **kwargs):
def put_at_tip(self, mob: Mobject, use_next_to: bool = True, **kwargs: Any) -> Self:
"""Puts the given mobject at the brace tip.

Parameters
Expand All @@ -153,7 +155,7 @@ def put_at_tip(self, mob: Mobject, use_next_to: bool = True, **kwargs):
mob.shift(self.get_direction() * shift_distance)
return self

def get_text(self, *text, **kwargs):
def get_text(self, *text: str, **kwargs: Any) -> Tex:
"""Places the text at the brace tip.

Parameters
Expand All @@ -172,7 +174,7 @@ def get_text(self, *text, **kwargs):
self.put_at_tip(text_mob, **kwargs)
return text_mob

def get_tex(self, *tex, **kwargs):
def get_tex(self, *tex: str, **kwargs: Any) -> MathTex:
"""Places the tex at the brace tip.

Parameters
Expand All @@ -191,15 +193,15 @@ def get_tex(self, *tex, **kwargs):
self.put_at_tip(tex_mob, **kwargs)
return tex_mob

def get_tip(self):
def get_tip(self) -> Point3D:
"""Returns the point at the brace tip."""
# Returns the position of the seventh point in the path, which is the tip.
if config["renderer"] == "opengl":
return self.points[34]

return self.points[28] # = 7*4

def get_direction(self):
def get_direction(self) -> Vector3D:
"""Returns the direction from the center to the brace tip."""
vect = self.get_tip() - self.get_center()
return vect / np.linalg.norm(vect)
Expand Down Expand Up @@ -238,7 +240,7 @@ def __init__(
font_size: float = DEFAULT_FONT_SIZE,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

buff: float = 0.2,
brace_config: dict | None = None,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
brace_config: dict | None = None,
brace_config: dict[str, Any] | None = None,

**kwargs,
**kwargs: Any,
):
self.label_constructor = label_constructor
super().__init__(**kwargs)
Expand All @@ -249,37 +251,51 @@ def __init__(
self.brace = Brace(obj, brace_direction, buff, **brace_config)

if isinstance(text, (tuple, list)):
self.label = self.label_constructor(*text, font_size=font_size, **kwargs)
self.label: VMobject = self.label_constructor(
*text, font_size=font_size, **kwargs
)
else:
self.label = self.label_constructor(str(text), font_size=font_size)

self.brace.put_at_tip(self.label)
self.add(self.brace, self.label)

def creation_anim(self, label_anim=FadeIn, brace_anim=GrowFromCenter):
def creation_anim(
self,
label_anim: type[Animation] = FadeIn,
brace_anim: type[Animation] = GrowFromCenter,
) -> AnimationGroup:
return AnimationGroup(brace_anim(self.brace), label_anim(self.label))

def shift_brace(self, obj, **kwargs):
# TODO: This method is only called from the method change_brace_label, which is
# not called from any location in the current codebase.
# TODO: The Mobject could be more specific.
Comment on lines +270 to +272
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. Btw, the suggestion is also removing the second TODO, but we can keep it if necessary. What do you exactly mean by that second TODO?

Suggested change
# TODO: This method is only called from the method change_brace_label, which is
# not called from any location in the current codebase.
# TODO: The Mobject could be more specific.

def shift_brace(self, obj: Mobject, **kwargs: Any) -> Self:
if isinstance(obj, list):
obj = self.get_group_class()(*obj)
self.brace = Brace(obj, self.brace_direction, **kwargs)
self.brace.put_at_tip(self.label)
return self

def change_label(self, *text, **kwargs):
# TODO: This method is only called from the method change_brace_label, which is
# not called from any location in the current codebase.
Comment on lines +280 to +281
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
# TODO: This method is only called from the method change_brace_label, which is
# not called from any location in the current codebase.

def change_label(self, *text: str, **kwargs: Any) -> Self:
self.label = self.label_constructor(*text, **kwargs)

self.brace.put_at_tip(self.label)
return self

def change_brace_label(self, obj, *text, **kwargs):
# TODO: This method is not called from anywhere in the codebase.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here: seems useful, but it's not documented.

Suggested change
# TODO: This method is not called from anywhere in the codebase.

def change_brace_label(self, obj: Mobject, *text: str, **kwargs: Any) -> Self:
self.shift_brace(obj)
self.change_label(*text, **kwargs)
return self


class BraceText(BraceLabel):
def __init__(self, obj, text, label_constructor=Tex, **kwargs):
def __init__(
self, obj: Mobject, text: str, label_constructor: type[Tex] = Tex, **kwargs: Any
):
Comment on lines +296 to +298
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as before:

Suggested change
def __init__(
self, obj: Mobject, text: str, label_constructor: type[Tex] = Tex, **kwargs: Any
):
def __init__(
self,
obj: Mobject,
text: str,
label_constructor: type[SingleStringMathTex | Text] = Tex,
**kwargs: Any,
):

super().__init__(obj, text, label_constructor=label_constructor, **kwargs)


Expand Down Expand Up @@ -320,7 +336,7 @@ def __init__(
point_1: Point3DLike | None,
point_2: Point3DLike | None,
direction: Vector3D | None = ORIGIN,
Comment on lines 336 to 338
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If any of point_1 and point_2 is None, the code below raises an error. direction should also be different from None.

Suggested change
point_1: Point3DLike | None,
point_2: Point3DLike | None,
direction: Vector3D | None = ORIGIN,
point_1: Point3DLike,
point_2: Point3DLike,
direction: Point3DLike = ORIGIN,

**kwargs,
**kwargs: Any,
):
if all(direction == ORIGIN):
line_vector = np.array(point_2) - np.array(point_1)
Expand Down Expand Up @@ -387,7 +403,7 @@ def __init__(
self,
arc: Arc | None = None,
direction: Sequence[float] = RIGHT,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
direction: Sequence[float] = RIGHT,
direction: Point3DLike = RIGHT,

**kwargs,
**kwargs: Any,
):
if arc is None:
arc = Arc(start_angle=-1, angle=2, radius=1)
Expand Down
7 changes: 6 additions & 1 deletion manim/mobject/text/tex_mobject.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from collections.abc import Iterable
from functools import reduce
from textwrap import dedent
from typing import Any

from manim import config, logger
from manim.constants import *
Expand Down Expand Up @@ -447,7 +448,11 @@ class Tex(MathTex):
"""

def __init__(
self, *tex_strings, arg_separator="", tex_environment="center", **kwargs
self,
*tex_strings: str,
arg_separator: str = "",
tex_environment: str = "center",
**kwargs: Any,
):
super().__init__(
*tex_strings,
Expand Down
3 changes: 0 additions & 3 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,6 @@ ignore_errors = True
[mypy-manim.mobject.opengl.opengl_vectorized_mobject]
ignore_errors = True

[mypy-manim.mobject.svg.brace]
ignore_errors = True

[mypy-manim.mobject.svg.svg_mobject]
ignore_errors = True

Expand Down