Skip to content

Add typing annotations to svg_mobject.py #4318

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

Merged
merged 5 commits into from
Jul 20, 2025

Conversation

henrikmidtiby
Copy link
Contributor

Overview: What does this pull request change?

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

@@ -509,11 +510,11 @@ def init_points(self) -> None:

def handle_commands(self) -> None:
all_points: list[np.ndarray] = []
last_move = None
last_move: np.ndarray = None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The np.ndarray type seems to work here. But is there any better? I don't think it should be Point3DLike.

@@ -121,10 +122,10 @@
self.color = color
self.opacity = opacity
self.fill_color = fill_color
self.fill_opacity = fill_opacity
self.fill_opacity = fill_opacity # type: ignore[assignment]

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class Warning

Assignment overwrites attribute fill_opacity, which was previously defined in superclass
VMobject
.
self.stroke_color = stroke_color
self.stroke_opacity = stroke_opacity
self.stroke_width = stroke_width
self.stroke_opacity = stroke_opacity # type: ignore[assignment]

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class Warning

Assignment overwrites attribute stroke_opacity, which was previously defined in superclass
VMobject
.
@@ -121,10 +122,10 @@ def __init__(
self.color = color
self.opacity = opacity
self.fill_color = fill_color
self.fill_opacity = fill_opacity
self.fill_opacity = fill_opacity # type: ignore[assignment]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The type annotations for the SVGMobject allows this value to be None, which is not compatible with the superclass VMobject. It seems to be fixed using some kind of default SVG value magic a few lines later.

Comment on lines +127 to +128
self.stroke_opacity = stroke_opacity # type: ignore[assignment]
self.stroke_width = stroke_width # type: ignore[assignment]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above.

@@ -435,7 +436,7 @@ def text_to_mobject(text: se.Text):
The parsed SVG text.
"""
logger.warning(f"Unsupported element type: {type(text)}")
return
return # type: ignore[return-value]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we get rid of this mypy ignore statement?

@henrikmidtiby henrikmidtiby marked this pull request as ready for review June 28, 2025 21:27
Copy link
Member

@behackl behackl left a comment

Choose a reason for hiding this comment

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

I think this is fine for a first round; systemic issues with allowed overwritten values will have to be treated differently anyways.

Thank you very much!

@github-project-automation github-project-automation bot moved this from 🆕 New to 👍 To be merged in Dev Board Jul 20, 2025
self.stroke_opacity = stroke_opacity
self.stroke_width = stroke_width
self.stroke_opacity = stroke_opacity # type: ignore[assignment]
self.stroke_width = stroke_width # type: ignore[assignment]

Check warning

Code scanning / CodeQL

Overwriting attribute in super-class or sub-class Warning

Assignment overwrites attribute stroke_width, which was previously defined in superclass
VMobject
.
@behackl behackl linked an issue Jul 20, 2025 that may be closed by this pull request
@behackl behackl merged commit fdb5cb9 into ManimCommunity:main Jul 20, 2025
21 checks passed
@github-project-automation github-project-automation bot moved this from 👍 To be merged to ✅ Done in Dev Board Jul 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants