Added type annotations to mobject/svg/brace.py #4309
Conversation
| # 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. |
| 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. |
There was a problem hiding this comment.
Same here: seems useful, but it's not documented.
| # TODO: This method is not called from anywhere in the codebase. |
| # 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. |
There was a problem hiding this comment.
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?
| # 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. |
| @@ -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).
| @@ -238,7 +240,7 @@ def __init__( | |||
| font_size: float = DEFAULT_FONT_SIZE, | |||
| buff: float = 0.2, | |||
| brace_config: dict | None = None, | |||
There was a problem hiding this comment.
| brace_config: dict | None = None, | |
| brace_config: dict[str, Any] | None = None, |
| @@ -70,14 +72,14 @@ def construct(self): | |||
| def __init__( | |||
| self, | |||
| mobject: Mobject, | |||
| direction: Vector3D | None = DOWN, | |||
| direction: Vector3D = DOWN, | |||
There was a problem hiding this comment.
Might this be typed as Point3DLike in the meantime?
| direction: Vector3D = DOWN, | |
| direction: Point3DLike = DOWN, |
| point_1: Point3DLike | None, | ||
| point_2: Point3DLike | None, | ||
| direction: Vector3D | None = ORIGIN, |
There was a problem hiding this comment.
If any of point_1 and point_2 is None, the code below raises an error. direction should also be different from None.
| point_1: Point3DLike | None, | |
| point_2: Point3DLike | None, | |
| direction: Vector3D | None = ORIGIN, | |
| point_1: Point3DLike, | |
| point_2: Point3DLike, | |
| direction: Point3DLike = ORIGIN, |
| def __init__( | ||
| self, obj: Mobject, text: str, label_constructor: type[Tex] = Tex, **kwargs: Any | ||
| ): |
There was a problem hiding this comment.
Same as before:
| 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, | |
| ): |
| @@ -387,7 +403,7 @@ def __init__( | |||
| self, | |||
| arc: Arc | None = None, | |||
| direction: Sequence[float] = RIGHT, | |||
There was a problem hiding this comment.
| direction: Sequence[float] = RIGHT, | |
| direction: Point3DLike = RIGHT, |
Overview: What does this pull request change?
Type annotations were added to the brace.py file.
Reviewer Checklist