manim
manim copied to clipboard
ManimCommunity
Improve and simplify Manim's color handling
Enhancement proposal
- Manim's colors are defined twice, once in an enum and once as constants. It seems like the Enum was introduced in #488 as an attempt to clean up the code, with constants having to be explicitly imported. However, for convenience the community eventually decided to import all the constants, and then to appease people's editors and linters the constants eventually had to be explicitly defined as well. I think we should either only define colors in an enum, or define them as constants, but not both. Seeing how the natural trend is to go towards convenience, I think they should be defined as constants. Furthermore, they should be instances of the
Colorclass from the colour module (or @marcin-serwin's fork). - We have many functions for converting between color representations that duplicate the functionality of existing, specialized libraries. Any uses of those should be replaced, probably by methods of
Color, and the functions eventually removed.
Additional comments
This sounds like a great proposal. Could I be assigned to this issue?
Before removing them completely do you think a deprecation warning should be used? I'm thinking of doing both versions
Thank you for your interest in contributing! I've assigned you to the issue.
Regarding the first point: I've realized that random_color relies on an object containing information on all preset colors, so perhaps it might not be desirable to remove such an object entirely. Perhaps Colors should be a list? I'm not sure.
Yes, a deprecation warning should be used where practical, but if it's not easily feasible then it can be left out. I'm not sure what you're referring to with "doing both versions".
I have already started kinda working on it in #2553 (The name could've been better 😅) Let me know if you would like to work on it with me together!
I'll look into variations we can do for the random_color. Some sort of way to gather all the different colors should be implemented. Are the colors limited to the ones currently implemented as constants or enums? Otherwise a random hexcolor code could maybe be used?
What I meant by the two versions was one with the deprecation warnings and one with those functions removed.
@Kiran-Raj-Dev I'll look into your PR!
We can only do this if Color can be pickled but for now it uses lambdas and thus cannot be serialized in to a file which hinders us from implemeting this feature
- https://github.com/ManimCommunity/manim/issues/2837
