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.

Sign up for GitHub

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

Jump to bottom

Typing mobject text #4092

Open

henrikmidtiby wants to merge 8 commits into ManimCommunity:main from henrikmidtiby:Typing_mobject_text

Contributor

henrikmidtiby commented Jan 6, 2025

Overview: What does this pull request change?

This pull request will add types to mobject/text/*.py

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

Links to added or changed documentation pages

Further Information and Comments

I indent to use this PR to share progress and get input on things that show up.

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

View reviewed changes

manim/mobject/text/code_mobject.py Outdated

+                                  # TODO
+                                  # Fix the mypy error from the next line of code.
+                                  # "Argument 1 to "append" of "list" has incompatible type "list[str]"; expected "str"  [arg-type]"
+                                  self.code_json[code_json_line_index].append([text, color])  # type: ignore[arg-type]

Contributor Author

henrikmidtiby Jan 6, 2025

I need input to solve the mypy issue mentioned here.

henrikmidtiby commented

View reviewed changes

manim/mobject/text/numbers.py

+                          # TODO: I am not sure about the type of mob_class.
+                          # I think it should be SingleStringMathTex, as that class has the _fontsize property.
+                          # But this seems to conflict with the default case, where it is set to VMobject.
+                          string_to_mob_map[string] = mob_class(string, **kwargs)  # type: ignore[assignment]

Contributor Author

henrikmidtiby Jan 6, 2025

I need input to solve the mypy issue mentioned here.

github-advanced-security bot found potential problems

View reviewed changes

manim/mobject/text/tex_mobject.py

@@ @@ -38,7 +41,7 @@ @@
               from manim.utils.tex import TexTemplate
               from manim.utils.tex_file_writing import tex_to_svg_file
-              tex_string_to_mob_map = {}
+              tex_string_to_mob_map: dict[str, VMobject] = {}

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable 'tex_string_to_mob_map' is not used.

henrikmidtiby commented

View reviewed changes

manim/mobject/text/tex_mobject.py

+                      # Give meaning full names to tex_strings_2 and tex_strings_3
+                      tex_strings_2 = [re.split("{{(.*?)}}", str(t)) for t in tex_strings]
+                      tex_strings_3 = sum(tex_strings_2, [])
+                      if len(tex_strings_3) > pre_split_length:

Contributor Author

henrikmidtiby Jan 6, 2025

I have used separate names for tex_strings, tex_strings_2 and tex_strings_3 as their types differ. Changing type of a variable like triggers a mypy error.

henrikmidtiby commented

View reviewed changes

manim/mobject/text/tex_mobject.py

                       part = self.get_part_by_tex(tex, **kwargs)
-                      return self.index_of_part(part)
+                      return self.index_of_part(part)  # type: ignore[arg-type]

Contributor Author

henrikmidtiby Jan 6, 2025

Is it possible to get rid of this type ignore?

henrikmidtiby commented

View reviewed changes

manim/mobject/text/tex_mobject.py

                       self.buff = buff
                       self.dot_scale_factor = dot_scale_factor
-                      self.tex_environment = tex_environment
+                      # TODO: See comment 10 lines above this

Contributor Author

henrikmidtiby Jan 6, 2025

Would it be ok to set the default tex_environment to "center"?

github-advanced-security bot found potential problems

View reviewed changes

manim/mobject/text/text_mobject.py

-                      code.code = mobject_without_dots
+                      # TODO:
+                      # error: "SVGMobject" has no attribute "code"  [attr-defined]
+                      code.code = mobject_without_dots  # type: ignore[attr-defined]

Check failure

Code scanning / CodeQL

Potentially uninitialized local variable Error

Local variable 'code' may be used before it is initialized.

henrikmidtiby commented

View reviewed changes

manim/mobject/text/text_mobject.py

-                      mobject = mobject.code
+                      # TODO:
+                      # error: Incompatible types in assignment (expression has type "MethodType", variable has type "SVGMobject")  [assignment]
+                      mobject = mobject.code  # type: ignore[assignment]

Contributor Author

henrikmidtiby Jan 8, 2025

Any ideas of how to handle this typing issue?

henrikmidtiby commented

View reviewed changes

manim/mobject/text/text_mobject.py

-                      code.code = mobject_without_dots
+                      # TODO:
+                      # error: "SVGMobject" has no attribute "code"  [attr-defined]
+                      code.code = mobject_without_dots  # type: ignore[attr-defined]

Contributor Author

henrikmidtiby Jan 8, 2025

Any ideas of how to handle this typing issue?

henrikmidtiby commented

View reviewed changes

manim/mobject/text/text_mobject.py

-                      file_name = self._text2svg(color.to_hex())
+                      parsed_color: ManimColor = ManimColor(color) if color else VMobject().color
+                      # TODO: Should _text2svg receive a str or a ManimColor?
+                      file_name = self._text2svg(parsed_color.to_hex())

Contributor Author

henrikmidtiby Jan 8, 2025

Any ideas of how to handle this typing issue?

henrikmidtiby commented

View reviewed changes

manim/mobject/text/text_mobject.py

-                              args["color"] = colors[i].to_hex()
+                              # TODO:
+                              # error: Item "float" of "ManimColor | float" has no attribute "to_hex"  [union-attr]
+                              args["color"] = colors[i].to_hex()  # type: ignore[union-attr]

Contributor Author

henrikmidtiby Jan 8, 2025

Any ideas of how to handle this typing issue?

henrikmidtiby commented

View reviewed changes

manim/mobject/text/text_mobject.py

-                              gradient = [ManimColor(color)]
+                              # TODO:
+                              # error: Incompatible types in assignment (expression has type "list[ManimColor]", variable has type "tuple[Any, ...]")  [assignment]
+                              gradient = [ManimColor(color)]  # type: ignore[assignment]

Contributor Author

henrikmidtiby Jan 8, 2025

Any ideas of how to handle this typing issue?

henrikmidtiby commented

View reviewed changes

manim/mobject/text/text_mobject.py

-                              color_gradient(gradient, len(word))
+                              # TODO:
+                              # error: Incompatible types in assignment (expression has type "list[ManimColor] | ManimColor | tuple[Any, ...]", variable has type "list[ManimColor] | ManimColor")  [assignment]
+                              color_gradient(gradient, len(word))  # type: ignore[assignment]

Contributor Author

henrikmidtiby Jan 8, 2025

Any ideas of how to handle this typing issue?

henrikmidtiby commented

View reviewed changes

manim/mobject/text/text_mobject.py

-                                  args["color"] = colors[i - start].to_hex()
+                                  # TODO:
+                                  # error: Item "float" of "ManimColor | float" has no attribute "to_hex"  [union-attr]
+                                  args["color"] = colors[i - start].to_hex()  # type: ignore[union-attr]

Contributor Author

henrikmidtiby Jan 8, 2025

Any ideas of how to handle this typing issue?

henrikmidtiby commented

View reviewed changes

manim/mobject/text/text_mobject.py

+                      settings = self._get_settings_from_t2xs(t2xs, default_args)  # type: ignore[arg-type]
+                      # TODO:
+                      # error: Argument 1 to "_get_settings_from_gradient" of "Text" has incompatible type "dict[str, Any | ManimColor | int | str | tuple[int, int, int] | tuple[float, float, float] | tuple[int, int, int, int] | tuple[float, float, float, float]]"; expected "dict[str, Iterable[str]]"  [arg-type]
+                      settings.extend(self._get_settings_from_gradient(default_args))  # type: ignore[arg-type]

Contributor Author

henrikmidtiby Jan 8, 2025

Any ideas of how to handle this typing issue?

henrikmidtiby commented

View reviewed changes

manim/mobject/text/text_mobject.py

-                              new_setting = self._merge_settings(setting, next_setting, default_args)
+                              # TODO:
+                              # error: Argument 3 to "_merge_settings" of "Text" has incompatible type "dict[str, Any | ManimColor | int | str | tuple[int, int, int] | tuple[float, float, float] | tuple[int, int, int, int] | tuple[float, float, float, float]]"; expected "dict[str, Iterable[str]]"  [arg-type]
+                              new_setting = self._merge_settings(setting, next_setting, default_args)  # type: ignore[arg-type]

Contributor Author

henrikmidtiby Jan 8, 2025

Any ideas of how to handle this typing issue?

henrikmidtiby marked this pull request as ready for review

January 8, 2025 20:20

behackl marked this pull request as draft

January 19, 2025 15:58

Contributor

chopan050 commented Jan 19, 2025

I would like to handle this PR eventually.

In the meantime, @henrikmidtiby, PR #4115 completely rewrote the Code mobject to make its code much cleaner. We already took care of its typing in that PR, so don't worry about code_mobject.py.

pre-commit-ci bot and others added 8 commits

January 20, 2025 17:11


          [pre-commit.ci] pre-commit autoupdate

c8a2228

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.9.1 → v0.9.2](astral-sh/ruff-pre-commit@v0.9.1...v0.9.2)


          Activated typechecking in mobject.text.*

00223a1

Error count: 156

# Conflicts:
#	mypy.ini


          Fixed typing errors in mobject/text/code_mobject.py

272ae04

# Conflicts:
#	manim/mobject/text/code_mobject.py


          Add type annotations to mobject/text/numbers.py

15bb770

# Conflicts:
#	manim/mobject/text/code_mobject.py


          Add type annotations to mobject/text/tex_mobject.py

48bbc75


          Return the object itself from init_colors

8ba69ea


          Fix issue.

0d7671a


          Add type annotations to mobject/text/text_mobject.py

ee181ac

henrikmidtiby force-pushed the Typing_mobject_text branch from 94382c5 to ee181ac Compare

January 20, 2025 21:04

Contributor Author

henrikmidtiby commented Jan 20, 2025

I would like to handle this PR eventually.

In the meantime, @henrikmidtiby, PR #4115 completely rewrote the Code mobject to make its code much cleaner. We already took care of its typing in that PR, so don't worry about code_mobject.py.
Great!
I have rebased the PR on top of the 0.19 branch.

henrikmidtiby marked this pull request as ready for review

January 20, 2025 21:15

This was referenced Jan 21, 2025

Typing progress tracker #3375

Open

Add typings to tex_mobject.py and numbers.py #4015

Open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet