Skip to content

Simplify the function remove_invisible_chars in text_mobject.py #4394

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 4 commits into
base: main
Choose a base branch
from

Conversation

henrikmidtiby
Copy link
Contributor

Overview: What does this pull request change?

This PR implements a small refactoring suggested by @chopan050 in text_mobject.py.
#4381 (comment)

The change simplifies the code layout so it is easier for mortals to understand.

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

@github-project-automation github-project-automation bot moved this to 🆕 New in Dev Board Aug 11, 2025
@henrikmidtiby henrikmidtiby marked this pull request as ready for review August 11, 2025 20:24
@behackl
Copy link
Member

behackl commented Aug 11, 2025

Just FYI, with #4392 the code-branch of this function is no longer needed at all. (The way it is implemented right now is actually also incompatible with the reworked Code mobject.)

Copy link
Contributor

@chopan050 chopan050 left a comment

Choose a reason for hiding this comment

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

Just FYI, with #4392 the code-branch of this function is no longer needed at all. (The way it is implemented right now is actually also incompatible with the reworked Code mobject.)

Thanks for the info, Benjamin! I didn't even realize that Code no longer has a code attribute.

With this new information, @henrikmidtiby:

  • since the Code branch does not apply anymore, because the invisible Dots would already be removed in the code with #4392, and
  • the Text branch does not make much sense (doing mobject = mobject[:] only transforms the Text into a VGroup, but the submobjects are the same and it does not affect the subsequent code),

I would say that remove_invisible_chars() should not really have any of those branches. In that case, it would only do the logic we wanted to move into exclude_dots_from_mobject(). We might as well delete the latter function and move all of its logic back into remove_invisible_chars():

def remove_invisible_chars(mobject: SVGMobject) -> VGroup:
    mobject_without_dots = VGroup()
    if isinstance(mobject[0], VGroup):
        for submob in mobject:
            mobject_without_dots.add(VGroup(k for k in submob if not isinstance(k, Dot)))
    else:
        mobject_without_dots.add(*(k for k in mobject if not isinstance(k, Dot)))
    return mobject_without_dots

@github-project-automation github-project-automation bot moved this from 🆕 New to 👀 In review in Dev Board Aug 13, 2025
@henrikmidtiby henrikmidtiby changed the title Extract function exclude_dots_from_mobject in text_mobject.py Simplify the function remove_invisible_chars in text_mobject.py Aug 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

3 participants