Skip to content

Need to re-implement custom LaTeX related methods for current config system. #199

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

Closed
Aathish04 opened this issue Jul 12, 2020 · 6 comments · Fixed by #223
Closed

Need to re-implement custom LaTeX related methods for current config system. #199

Aathish04 opened this issue Jul 12, 2020 · 6 comments · Fixed by #223
Labels
pr:bugfix Bug fix for use in PRs solving a specific issue:bug

Comments

@Aathish04
Copy link
Member

As of now, both ExampleClassScene and ExampleFileScene in example_scenes/customtex.py fail, throwing a LaTex error converting to DVI each time.

I believe this is because of the removal of register_tex_template and initialize_tex from config.py when the config system was revamped.

These methods need to be reimplemented, and customtex.py needs to be edited accordingly to fit the new config system.

Mentioning @leotrs since he rewrote config.
Mentioning @PhilippImhof and @cobordism for the LaTeX part.

@Aathish04 Aathish04 added the pr:bugfix Bug fix for use in PRs solving a specific issue:bug label Jul 12, 2020
@PhilippImhof
Copy link
Member

That's bad. I remember having had a discussion with @leotrs about it in the chat....(searching)...on June 4th. It ended with Leo saying

so in that case, I know where to put it :slight_smile:

I am currently away, so I won't be able to look into it before more or less the end of July. It might take me some time to dive into the new structures put in place with #98. I would appreciate if @leotrs could have a look.

@leotrs
Copy link
Contributor

leotrs commented Jul 21, 2020

At the top of the customtex.py file there is a comment that explains the usage of said file. I'll leave my thoughts one by one.

# 1. manim customtex.py ExampleFileScene -pl
#       --> should fail, because \vv is not defined

There shouldn't be a usage that intentionally fails in the examples folder. May I ask one of @Aathish04 @PhilippImhof to write a test for this instead? (A test that checks that the right exception is thrown.) If you need help, ask @huguesdevimeux. Once the test is merged, please remove this usage from this file.

# 2. manim customtex.py ExampleFileScene --tex_template custom_template.tex -pl
#       --> should succeed as custom template includes package esvect (which defines \vv)

This works for me with the latest master from today. I can do either manim example_scenes/customtex.py ExampleFileScene --tex_template example_scenes/custom_template.tex -pl from the main manim folder, as well as manim customtex.py ExampleFileScene --tex_template custom_template.tex -pl from the manim/example_scenes/ folder.

# 3. manim customtex.py ExampleClassScene -pl
#       --> should succeed as the package esvect is included in template object

This required a single line change, from config.register_tex_template(tpl) to config['tex_template'] = tpl.

Now both seem to work for me. Just pushed my changes. Please pull and confirm, or let me know if I missed anything.

@PhilippImhof
Copy link
Member

First of all: Thank you very much for the changes. I have yet to familiarize myself with the new (and improved) structures.

At the top of the customtex.py file there is a comment that explains the usage of said file. I'll leave my thoughts one by one.

# 1. manim customtex.py ExampleFileScene -pl
#       --> should fail, because \vv is not defined

There shouldn't be a usage that intentionally fails in the examples folder. May I ask one of @Aathish04 @PhilippImhof to write a test for this instead? (A test that checks that the right exception is thrown.) If you need help, ask @huguesdevimeux. Once the test is merged, please remove this usage from this file.

I do not fully agree: People might see the file and try to call run it that way, e.g. run customtex.py without any parameters, see that there are two scenes in it and try both of them. They will then wonder, why that damn stuff does not work. Probably not developers, but users. So that line is not there as a usage that intentionally fails, but as an explanation why it is not possible to run this specific scene from customtex.py like this.

I suggest rewording the text along the lines of: call ExampleFileScene like this or ExampleClassScene like that; if you call ExampleFileScene without giving a sensible template (i.e. one that defines the \vv macro that is being used in the scene), it will fail.

What do you think?

@leotrs
Copy link
Contributor

leotrs commented Jul 22, 2020

The thing is that this should be documented in the documentation, not in the examples file. For now, since there is no central documentation, I'd suggest at least changing the name of the class from ExampleFileScene to something more descriptive such as NeedsExternalCustomTex or even WillCrashUnlessUsingExternalTex. Perhaps even adding some code to the class as

class ExampleFileScene(Scene):
    def construct(self):
        assert config['custom_tex'] is not None, Exception('requires custom tex')
        text = TexMobject(r"\vv{vb}")
        self.play(Write(text))

@cobordism
Copy link
Member

Demo time:

Like I have done previously for 3b1bmanim, I have added support in manimcommunity for all tex fonts.
Video is here: https://vimeo.com/441072916

code to include is here:
https://gitlab.com/co-bordism/manim/-/tree/all_tex_fonts_community

code together with example scene that renders the above video is here:
https://gitlab.com/co-bordism/manim/-/tree/all_tex_fonts_community_demo

@Aathish04
Copy link
Member Author

Aathish04 commented Jul 23, 2020

Demo time:

Like I have done previously for 3b1bmanim, I have added support in manimcommunity for all tex fonts.

@cobordism
I've opened PR #215 with your code.
Would you mind creating a new issue for the changes you've made, since this one's talking about how to document the Customtex Tex ExampleScene?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:bugfix Bug fix for use in PRs solving a specific issue:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants