Finally fixing the Color situation

[MrDiver]

To fix

As discussed on discord

The color situation in Manim is pretty janked right now and there are 2-3 different ways to manage Color. We do not really need all that duplicate functionality so i think we settled now on reworking the Enum in color.py to be the source of truth again. Making it Colors(str, Enum) to not break current functionality (but also benchmark that at first before doing that change because it could have drastic performance impact) and then so that IDE doesn't cry we add a .pyi file with the exported colors so that autocomplete works and then ditching colour.py because it is not compatible with pickle

• https://github.com/ManimCommunity/manim/issues/2451

Enhancement proposal

* Manim's colors are defined twice, [once in an enum](https://github.com/ManimCommunity

/manim/blob/main/manim/utils/color.py#L33-L260) and once as [constants](https://github.com /ManimCommunity/manim/blob/main/manim/utils/color.py#L286-L366). It seems like the Enum was introduced in # New colour enums- Try2 #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 Color class from the colour module (or @marcin-serwin's fork).

* We have many functions for [converting between color representations]

(https://github.com/ManimCommunity/manim/blob/main/manim/utils/color.py#L453-L478) 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

Related

• https://github.com/ManimCommunity/manim/pull/2606
• #2605

[kilacoda]

I'd be willing to put some time and work on this.

So if I understand correctly, we want to make Colors(str,Enum) so that we could access members in the following way, without having to explicitly call the .value attribute:

some_mobject.set(color=Colors.red)

but we also want to keep the existing syntax across most Manim scripts:

some_mobject.set_color(RED)

and the way we do this is somewhat as follows:

class Colors(str, Enum):
    ...
    red = "#FC6255"
    ...

RED = Colors.red

and also, we have to get rid of all references to colour.Color in the utils/color.py module as well as throughout the library.

CMIIW please, I want to be sure what we're working on here before we get started.

[MrDiver]

@kilacoda I didn't test if exporting directly would work but my thought was more in the realms of exporting the Enum values directly. I don't know what that would look like but a first step would be renaming the colors in the Colors Enum to all caps. If it works to export those values with __all__ we can just remove the RED = Colors.RED part.

The rest is spot on

As long as it doesn't break existing behavior it should be fine. The only things that should change in the end are the imports and color.py

Maybe you can also do something along the lines of from Colors import * but that would also need some investigation.

If you want to test if this works just go ahead you could build a mini example with init files to test the behavior. If it works great and if it doesn't just write down how you would go about doing it.

[MrDiver]

@kilacoda There was something with generating constants from the Enum directly which might also be a viable option but requires us to add a pyi file but @naveen521kk or @behackl probably know more about working with these kinds of files or where the corresponding generation code is.

[ad-chaos]

One way to export the enum is by adding a classmethod like

@classmethod
def export_to(cls, namespace):
    namespace.update(cls.__members__)

Found it here

Not sure if this is nice, but then again we inject so many things into the global namespace anyway, so it doesn't seem that bad. 🤷

[kilacoda]

One way to export the enum is by adding a classmethod like

@classmethod
def export_to(cls, namespace):
    namespace.update(cls.__members__)

Found it here

Not sure if this is nice, but then again we inject so many things into the global namespace anyway, so it doesn't seem that bad. 🤷

I like this solution tbh, since these constants are required basically everywhere anyways, though I doubt it'll solve our problems with editor autocompletion unless we make a .pyi file.

[behackl]

It definitely is worth thinking more about an alternative to the colour package that we use; it hasn't been updated or actively maintained for 5 years at this point.

Essentially, it boils down to the question of whether we actually need a dedicated class handling colors, or whether using our (or some other libraries) conversion functions is easier or more convenient. Maybe this is something that we could poll on Discord?