Automatically import Plugins

[naveen521kk]

Motivation

Specify plugins using CLI and config file
This was discussed on Discord

• Automatically import plugins so that they that users don't need to do them.
• Specifying plugins using config files as well as CLI.
• Slight changes in the entry point procedure

Overview / Explanation for Changes

With manim_plugintemplate it can be used as

from manim import *
class Plugin(Scene):
    def construct(self):
        dot=DotGrid()
        self.play(FadeIn(dot))

previously this needs manual import of manim_plugintemplate

Oneline Summary of Changes

- Automatically Import Plugins (:pr:`967`)

Testing Status

Locally worked with a slight modification to plugin_template. Will make a PR there also. See ManimCommunity/manim-plugintemplate#6

cc @jsonvillanueva

Acknowledgements

• I have read the Contributing Guidelines

Reviewer Checklist

• Newly added functions/classes are either private or have a docstring
• Newly added functions/classes have tests added and (optional) examples in the docs
• Newly added documentation builds, looks correctly formatted, and adds no additional build warnings
• The oneline summary has been included in the wiki

[friedkeenan]

I'm not very knowledgeable about how plugins work and all that, so please bear with me if that shows overmuch. Generally I don't think I'm on board with plugins sharing manim's namespace; I think their own namespaces should be preserved somehow, but if that's not possible then I can live with that.

[friedkeenan]

Do the plugins' namespaces have to be eradicated? Because I'd rather they be preserved

[naveen521kk]

I would like that to happen :)
Because users can also import plugins manually if there want its namespace.

[jsonvillanueva]

I think the result of the discussion on Discord was that we should use a config file to determine the default plugins that are imported/used (as well as manually using --plugins to specify which plugins to use/import in the entry point)

[friedkeenan]

Yeah, I'm just not a fan of the possible namespace thrashing

[jsonvillanueva]

I don't mind the potential namespace thrashing (some plugins like the manim-onlinetex @Aathish04 rely on this); however, I do mind the syntax highlighting issue this presents and I think others would too -- which is why I would opt for CLI to change the plugins/__init__.py file which wouldn't hinder syntax highlighting

[naveen521kk]

for CLI to change the plugins/init.py file which wouldn't hinder syntax highlighting

That is bad practice. Some package managers checksum the files it installs and doing so will break it. And other times things installed from the package manager isn't usually writable and it would need to run in sudo mode which may create unnecessary problems.

[friedkeenan]

Tried searching for a cleaner way to set local variables with dynamic names, unfortunately could not find one, and exec seems to be the cleanest way. I'm not sure of the purpose of this review comment, I guess a general lamentation and a note to any others who might know of a cleaner way. Writing to locals(), vars(), or globals() is explicitly discouraged by the python docs, so that's not an option. I'm not sure if maybe preserving plugins' namespaces would make this unnecessary?

[naveen521kk]

I will add a test for these soon.

[naveen521kk]

This is ready with a Test :)

[friedkeenan]

Personally not too big a fan of the / operator on paths, I like to make it more explicit like Path(path, "manim.cfg") or you could use the .joinpath method, but the way it currently is is still fine.

I'd point out the other places but I accidentally hit "Add single comment" on my first comment and now feel it might be weird to start a whole new separate review but also don't want to spam this so yeah

[naveen521kk]

I like this way though...

[friedkeenan]

I mean yeah fair enough I'm just not too big a fan of it, I'm not at all staunchly against the / operator here. Feel perfectly free to not change anything unless maybe someone else wants to throw in an opinion I guess

[jsonvillanueva]

The rest of the library seems to use
Path(your_file_name) / "rest" / "of" / "path"

So I would just go for consistency here and do whatever is done mostly elsewhere in the codebase. Although, in general we should just have one style for these sorts of things.

Edit: This might not be mostly done elsewhere, but it is another way I've seen this done in the library.
In particular see: manim_directive.py, utils.py, sounds.py, make_and_open_docs.py, test_copy.py, conftest.py, and text_mobject.py
In other files, it seems to use @naveen521kk preferred syntax: test_img_and_svg.py and test_cli_flags.py.

[jsonvillanueva]

Just tested and it works:

However, keep in mind that using this feature will break syntax highlighting in the VS Code editor. If syntax highlighting is necessary, it's better to instead have this file (which defeats the purpose of this PR):

from manim import *
from manim_plugintemplate.mobjects.dotgrid import DotGrid

class MyScene(Scene):
    def construct(self):
        grid = DotGrid()
        self.add(grid)
        self.wait()