completely rewrote the TeX template model

[PhilippImhof]

I have completely rewritten the code for managing the TeX template:

• Full backwards compatibility: if you do nothing, everything will work as before, the old default template will be used for LaTeX or XeLaTeX
• New command line option --tex_template allows for a custom tex file
• New class TeXTemplate starting with the current default values for maximum backwards compatibility.
• New class has methods to append or prepend custom packages and/or custom preamble commands.
• Added method register_tex_template allows to register a new or modified template

Example

class ExampleScene(Scene):
    def construct(self):
        tpl=TeXTemplate()
        tpl.append_package(["esvect",["f"]])
        register_tex_template(tpl)

        text=TextMobject(r"$\vv{vb}$")
        self.play(Write(text))

As you may have guessed, the syntax for packages is

• append_package("package") if you do not have any options
• append_package(["package",["option1","option2=value2"]]) if you do have options to give

When using the command line approach with a custom template file, the simpler class TeXTemplateFromFile is used. It does not allow adding/removing packages from within the scene. However, I do not think there is a need for it, because if one specifies a custom template file, we can surely assume they will not need to dynamically adapt it later on.

[PhilippImhof]

This PR has been sent to 3b1b's repository a few months ago, but never got merged as (probably) Grant did not consider this useful to his own projects. It might be different in a community maintained version of manim that is used by many people in different situations.

See 3b1b/manim#745 for discussion

[PgBiel]

note: I didn't analyze the code yet, just pointing out a few ideas that stand out

[yoshiask]

I don't think this is something to address immediately in this PR, but we should consider putting this somewhere other than constants.py. By definition, dynamically loading TeX templates is not constant.

[PhilippImhof]

@PgBiel I added documentation and (hopefully) cleaned up the list of commits. Please do not hesitate to suggest further improvements, if necessary.

[PgBiel]

@PhilippImhof you might want to add the parameters, their types and the return types of each function to the documentation. I'll then modify it to use our format for you

[PgBiel]

Just a few QoL suggestions

[PhilippImhof]

@PgBiel Thanks for those improvements.

[PgBiel]

i'll commit those bugs i left off

[PhilippImhof]

Shame on me, I didn't even see it.... Thanks

[leotrs]

I'm removing myself as a requested reviewer and requesting @yoshiask instead, since they seem to have thoughts about this PR.

[PhilippImhof]

@yoshiask Certainly true. On the other hand, the TeX template will probably never ever be changed in the middle of an animation. Once initialized and set up, it will remain constant at least for one scene. (The only reason I can see for changing the template in between would be that one absolutely needs a certain package or definition in one scene, but cannot have it in the remainder of the animation because of a bad conflict.)

[PgBiel]

Note that we also need to replace all print instances here with logging.<x> when resolving conflicts

[PhilippImhof]

Good point. I will have a look at this tomorrow.

[PhilippImhof]

Commit 550c8b4 (and only this one) is untested, as I was unsure about the right way to pull from this master branch without having all those commits in the PR again. Also, I did not add the necessary imports, as they are already done in previously merged commit 00a786e in PR #59

[PhilippImhof]

As nothing has happened for a few days, I wondered whether I could do anything to help in moving on? With the refactoring going on, merging will become more difficult.

I think this PR would be useful for those who need additional packages (typically people who do not live in the US or other English speaking countries, like me) and it comes with no risk.

[leotrs]

@PhilippImhof thanks for the bump :) Just approved it. Now you need to take care of the merge conflicts. Please post here if you get stuck there (as quite a bit is happening on the repo these days).

[PhilippImhof]

@PhilippImhof Now you need to take care of the merge conflicts. Please post here if you get stuck there (as quite a bit is happening on the repo these days).

Thanks for approving the PR. I'm afraid I'm already stuck. I do not see the conflicts, I just see: "This branch has conflicts that must be resolved. Only those with write access to this repository can merge pull requests."

Should I just pull the last version of ManimCommunity/master, rebase my branch, solve the conflicts locally and push another commit? (Rebasing is still somewhat difficult to me, it seems I still haven't come to a very deep understanding of this part of git.)

[leotrs]

Should I just pull the last version of ManimCommunity/master, rebase my branch, solve the conflicts locally and push another commit?

An alternative is to directly try to pull ManimCommunity/master into your customtex branch, and resolve the conflicts. No rebasing necessary. Then push a new commit.

(Rebasing is still somewhat difficult to me, it seems I still haven't come to a very deep understanding of this part of git.)

Same, rebases are scary :)

[PhilippImhof]

Thanks @leotrs

An alternative is to directly try to pull ManimCommunity/master into your customtex branch, and resolve the conflicts. No rebasing necessary. Then push a new commit.

I will try. Just to make sure: Won't this bring all those other commits into this PR again, like I had it at the very beginning?

Also, won't this trigger another need for approvals? (What I mean: While waiting for new approvals, development in master goes on and new merge conflicts will probably appear.)

[leotrs]

If you are worried about all the commits, you can squash them! (Though I guess this will require a rebase...) In any case, as long as there are no conflicts between your branch and current master, I say don't worry about those commits.

[PgBiel]

Also, won't this trigger another need for approvals? (...)

No

[PhilippImhof]

Unfortunately, I am still stuck. I pulled the community's master, but for now, manim has stopped working. (ModuleNotFoundError: No module named 'manim' when trying to render the basic example scene.)

This happens even in a brand new directory populated with git clone https://github.com/ManimCommunity/manim.git.

[Aathish04]

Unfortunately, I am still stuck. I pulled the community's master, but for now, manim has stopped working. (ModuleNotFoundError: No module named 'manim' when trying to render the basic example scene.)

This happens even in a brand new directory populated with git clone https://github.com/ManimCommunity/manim.git.

Have you run pip install -e . when inside the cloned repository?

[PhilippImhof]

Have you run pip install -e . when inside the cloned repository?

Yes, sorry for not mentioning it. I was just editing my comment when the notification of your question popped up...

[Aathish04]

Have you run pip install -e . when inside the cloned repository?

Yes, sorry for not mentioning it. I was just editing my comment when the notification of your question popped up...

Have you checked if manim is present in your installed packages with pip freeze? It should be there...

[PhilippImhof]

Thanks for taking the time to help me.

Have you checked if manim is present in your installed packages with pip freeze? It should be there...

$ pip freeze | grep manim
-e git+https://github.com/ManimCommunity/manim.git@a29eabdf877442d3bee89a1d66bd0ff721484221#egg=manimlib

[Aathish04]

$ pip freeze | grep manim
-e git+https://github.com/ManimCommunity/manim.git@a29eabdf877442d3bee89a1d66bd0ff721484221#egg=manimlib

Thats very strange indeed. How are you running manim in your scripts and in the command line?

Importing manim in the file with from manim import * and running from the command line with manim <path to file> <flags> works for me. Is your method of use different?

[PhilippImhof]

Importing manim in the file with from manim import * and running from the command line with manim <path to file> <flags> works for me. Is your method of use different?

That's exactly what I do. example_scenes/basic.py has from manim import * and I invoke it just like you. (I used to invoke it in the old way python3 manim.py <file> <scene> <flags> before, but I obviously prefer the new way.)

[Aathish04]

Importing manim in the file with from manim import * and running from the command line with manim <path to file> <flags> works for me. Is your method of use different?

That's exactly what I do. example_scenes/basic.py has from manim import * and I invoke it just like you. (I used to invoke it in the old way python3 manim.py <file> <scene> <flags> before, but I obviously prefer the new way.)

Do you have any other manim installations that might conflict with this? If you aren't able to solve it, it might be worth filing an issue...

[PhilippImhof]

Do you have any other manim installations that might conflict with this? If you aren't able to solve it, it might be worth filing an issue...

I suspect so, because if it was a problem in the repo, someone would have noticed way earlier. I will try to clean up the whole mess. It might be conflicting with the original 3b1b manim installation I had. Once I find out, I will post an update.

[leotrs]

Thanks, and sorry for the mess. When/if you get unstuck again, please post here the full traceback and error message :)

…

On Thu, May 28, 2020, 5:53 AM Philipp Imhof ***@***.***> wrote: Do you have any other manim installations that might conflict with this? If you aren't able to solve it, it might be worth filing an issue... I suspect so, because if it was a problem in the repo, someone would have noticed way earlier. I will try to clean up the whole mess. It might be conflicting with the original 3b1b manim installation I had. Once I find out, I will post an update. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#48 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAILYAHPRGQGETIZYMSHM73RTYYBVANCNFSM4NG2EEZQ> .

[PhilippImhof]

So I got it all working again, by deleting all virtualenv stuff, removing everything that was related to manim and installing it all again.

I also managed to resolve all conflicts, there have been quite a lot of changes. Now, before I push my changes to this PR, I just want to make sure not to blow things up:

GitHub Desktop shows me 233 commits to push, which of course includes all the commits I pulled from master. Can I be sure that it will not cause any problems if I just push? (I tried squashing, but it resulted in strange conflicts.)

[leotrs]

Are you sure those 233 are with respect to ManimCommunity/manim:master? GitHub is showing only 16 commits:

[leotrs]

Uhm wait, I guess you didn't push the latest commit yet, right? If that would imply the 233 commits, then I'd consult @eulertour.

[PhilippImhof]

I am not sure of anything. But as I can only push to my own repository, they should be with respect to this one. However, I just do not know GitHub well enough to know, whether it is smart enough to leave them out in the PR.

Correct, I did not push after resolving the merge, because I did not want to flood the PR with "unnecessary" commits.

[PhilippImhof]

To be more precise: git log origin/master..HEAD shows me all commits starting with

f9bb278d Add python -m for windows pytest

up to

a29eabdf (upstream/master) Merge pull request #99 from [...] remove_unnecessary_files

and then finally my last three commits

d7381997 (HEAD -> customtex) Resolved merge conflicts
64d774ae Merge branch 'master' into customtex
a7e42f54 (master) Merge branch 'master' of https://github.com/ManimCommunity/manim

It is 249 lines, probably the 16 already in this PR plus the 233 to push (of which 230 are imported from here)

[PgBiel]

You can prob try pushing to your branch and, if bad stuff happens, you can revert to the current state

[PhilippImhof]

Finally....

[PhilippImhof]

It is all cleaned up now. I am sorry, but as a consequence, proper attribution of and credit for @PgBiel's improvements is gone.