manim
manim copied to clipboard
ManimCommunity
Using pluggy for managing Plugins
Enhancement proposal
Pluggy: https://pluggy.readthedocs.io/
Currently how out plugin system works is
- Get the list of plugins from the user config file
- Add them to the global namespace, so when the user does
from manim import *the plugin will also be imported. - Has no possible way to extend parts of the library without monkey patching or making a new subclass out of the original one. See how
manim-onlinetexdoes this currently, https://github.com/ManimCommunity/manim-onlinetex/blob/38ed76c55f387e9205beeb28307a634b46691df1/src/manim_onlinetex/manim_onlinetex.py#L109
With pluggy in place, we can decide the parts of the library which can be extendable without subclassing, see an example. https://pluggy.readthedocs.io/en/latest/#a-toy-example And it feels like we shouldn't reinvent the wheel rather than using something which is already available.
Additional comments
This would kinda require all the plugins to update to use pluggy. Let the previous implementation of importing to global namespace stay, for backward compatibility.
Proof of concept
- #2635
I think the drawback from learning a new tool will outweigh the potential benefit from using a solution outside the standard library, especially this early on. I think following the instructions from python's documentation will probably work for what we need.
The tiny amount of plugins would make this the best time to make this change if we ever do,
I think following the instructions from python's documentation will probably work for what we need.
That is already implemented and is working, using https://packaging.python.org/guides/creating-and-discovering-plugins/#using-package-metadata
I think the drawback from learning a new tool will outweigh the potential benefit from using a solution outside the standard library, especially this early on
I think for the number of plugins we have currently, it is better and easier to move to something which is well tested and has more features. With the current implementation, people can either subclass or monkey patching, which can sometimes become problematic, see https://pluggy.readthedocs.io/en/latest/#why-is-it-useful.
One thing that I'd be cautious of is the idea of requiring changes to the main code in order to accommodate new plugins. From that page,
The pluggy approach puts the burden on the designer of the host program to think carefully about which objects are really needed in a hook implementation.
If we do want to go with this approach we should be careful not to have a situation where people who develop plugins are continually making PRs to add hooks for them.
If #1131 goes through, and we do trim the fat, it should not be a big issue to add hooks. Esessialy with our class inheritance structure.
If we do want to go with this approach we should be careful not to have a situation where people who develop plugins are continually making PRs to add hooks for them.
Could be painful yeah, but if we already have good implementation at first, this won't happen. Also, this would help us implemented new Renderer's, Mobjects, Animations very easily. As in that's what you are telling in #1131. Actually, speaking it would hassle to implements the things you told in #1131 (about removing extra code), without this implementation.
@eulertour Going with what was said above, I think in the code move that we will make most of the needed hooks. I say someone should make a pull for this so we can see the changes in reality.
If we want to put this in hold then we should specify what it means to move to opengl. Does this mean cairo rendering needs to be removed? Or just not used by default or deprecated or something like that.
I'm sorry I meant to put that in #1131. I still want this to go through now, as I think it will help plugin devs (Me :P) with their plugins.