-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Rewrite MathTex to make it more robust regarding splitting #4515
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
base: main
Are you sure you want to change the base?
Rewrite MathTex to make it more robust regarding splitting #4515
Conversation
updates: - [github.com/astral-sh/ruff-pre-commit: v0.14.7 → v0.14.8](astral-sh/ruff-pre-commit@v0.14.7...v0.14.8) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…of curly braces around the extracted part
… braces for substrings_to_isolate to work
…ble curly braces in the example. This would otherwise trigger MathTex to split the string at that location.
|
I have located a minor issue related to the "Using text" guide. In the example |
behackl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great and I love the much more robust implementation. Kudos and thank you!
Here are some more suggestions:
- It might be nice to have some sort of validation in
SVGMobject.__init__that checks whether all ids from the SVG are contained in the id map at the end of initialising (and log warnings if some are missing?) - The PR description should mention removal/renaming of
get_parts_by_tex,index_of_part_by_tex
Additionally, my review clanker suggests adding two properties to MathTex:
@property
def _substring_matches(self) -> list[tuple[str, str]]:
"""Return only the 'ss' (substring_to_isolate) matches."""
return [(tex, id_) for tex, id_ in self.matched_strings_and_ids if id_.endswith("ss")]
@property
def _main_matches(self) -> list[tuple[str, str]]:
"""Return only the main tex_string matches."""
return [(tex, id_) for tex, id_ in self.matched_strings_and_ids if not id_.endswith("ss")]in order to make break_up_by_substring more readable,
for tex_string, tex_string_id in self._main_matches:
mtp = MathTexPart()
mtp.tex_string = tex_string
mtp.add(*self.id_to_vgroup_dict[tex_string_id].submobjects)
new_submobjects.append(mtp)Not sure whether that is worth it, and/or whether 'ss' should perhaps be made more explicit (I'd like that) -- but other than these I don't really have any substantial feedback. I like it.
Left a few more inline comments, please do take a look!
| :class:`~.MathTex` into substrings automatically and isolate the ``x`` components | ||
| into individual substrings. Only then can :meth:`~.set_color_by_tex` be used | ||
| to achieve the desired result. | ||
| If one of the ``substrings_to_isolate`` is in a sub or superscript, it needs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we attempt to strengthen the parser to make this not necessary? Perhaps not at first.
| for parent_name in vgroup_stack[:-1]: | ||
| vgroups[parent_name].add(mob) | ||
| except Exception as e: | ||
| print(e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logger.error with a bit more context?
| mtp.add(*self.id_to_vgroup_dict[tex_string_id].submobjects) | ||
| new_submobjects.append(mtp) | ||
| except KeyError as e: | ||
| print(f"KeyError: {e}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, more context + logger?
| print(f"KeyError: {e}") | |
| logger.error(f"MathTex: Could not find SVG group for tex part '{tex_string}' (id: {tex_string_id}). Using fallback to root group.") |


Overview: What does this pull request change?
This PR changes a large part of the implementation of the MathTex class.
The reason is to deal with the issues related to splitting the tex string and then try to match the generated objects with parts of the original tex string.
The idea for the PR is based on this discussion on discord.
https://discord.com/channels/581738731934056449/1406977902788218892
Motivation and Explanation: Why and how do your changes improve the library?
This PR changes how
MathTexobjects are created / rendered.To illustrate the changes in the process, please look at the following example
of calling
MathTex.The first step in rendering the MathTex equation, is to join the three separate strings to a single string and add some markup to the string that will allow manim to extract certain parts of the equation afterwards. Each of the three input strings are put in the following markup.
This ends up looking like the following:
This allows us to access a named group in the formed svg file corresponding to each of the three tex strings.
For each string, the elements of
substrings_to_isolateis also located, and marked up with a similar markup.E.g. the
ain the equation ends up like this, where the element is placed in the named group 'unique000ss'.All there marked up strings are joined to the following string (newlines were added to increase readability).
This string is then saved to a
texfile and converted to first a.dviand then a.svgfile using thelatexanddvisvgmcommand line tools.Finally the svg file is loaded by a
SVGMobjectfrom which the named groups can be accessed as demonstrated here.The full code for this example is provided here
and it renders as follows

Links to added or changed documentation pages
Further Information and Comments
This PR is an extension of #4473 "Maintain groups when importing svg files"
Reviewer Checklist