Manim: Using classmethods for constants

[huguesdevimeux]

I don't know if it would be a good idea, or if this has been already brought up (I think @leotrs mentioned a similar idea in #310 ).

The idea would be to change the way all the constants are implemented into something that would be similar to namespaces.
My idea would be, for example, for the colors, to do something like :

class Color : 
    ...
    @classmethod
    def red(self):
        return <color red>

Thus, one could access the red colour by simply doing Color.red() instead of the ugly RED

Similar reasoning could be applied for every item within constants.py. So we would have, for example, Message.not_setting_font().
We could even go further, by implementing all constants utils within these classes. (for example, utils.interpolate_color)

A major advantage would be that everything would be much better organized and more, I think, clean. The constants would be well arranged by classes (and so we could get auto-completion ;D), On the other hand, it would be a little bit more long to use, as you would have to access to Color every time.

NOTE : I took inspiration on how colors are implemented on the discord python API, but I can"'t find a link to the file :/

[leotrs]

I agree with your proposal in general. Three points I want to make.

1. Instead of using class methods Color.red(), we should use either (a) class attributes Color.red, or the actual SimpleNamespace class.

2. Right now you can import single colors as from manim import RED. Putting all colors inside a class or namespace means you have to import the whole class/namespace from manim import Color or from manim import colors. This is fine by me but it will break basically every single scene in existence. We have to decide whether we want to do this or just deprecate the old-style colors and then remove them in a future release.

3. The particular case of Color is interesting to me. After reading utils/color.py, I'm convinced that we should create our own class that inherits from colour.Color. This class would support alpha channels (which are currently handled haphazardly), produce all output as numpy arrays (so no more np.array() calls all over the codebase) and it may also contain pre-defined colors as classmethods/attributes as you suggest. What I'm saying is that we do not necessarily need to give colors the same treatment as other constants. But let's keep this for a future issue.

[kilacoda]

An enum might be useful for colors:

from enum import Enum

class Colors(Enum):
    white = "#000000"
    black = "#FFFFFF"
    ...

## use Colors.<color>.value internally
print(Colors.white.value) ## "#000000"

[PgBiel]

Yes, that's also what I proposed doing (enum for colors)
However, can we please switch to colors with ints? Strings require parsing. Let's delegate that job to Python

so your example would be

from enum import Enum

class Colors(Enum):
    white = 0x000000
    black = 0xFFFFFF
    ...

## use Colors.<color>.value internally
print(Colors.white.value) ## 0x000000

[PgBiel]

1. Instead of using class methods `Color.red()`, we should use either (a) class attributes `Color.red`, or the actual [SimpleNamespace](https://docs.python.org/3/library/types.html#types.SimpleNamespace) class.

Yeah, this is what class attributes are for; note that attrs allows for this without overlapping with the user-specified attrs by using the ClassVar type from typing.

2. Right now you can import single colors as `from manim import RED`. Putting all colors inside a class or namespace means you have to import the whole class/namespace `from manim import Color` or `from manim import colors`. This is fine by me but it will break basically every single scene in existence. We have to decide whether we want to do this or just deprecate the old-style colors and then remove them in a future release.

deprecate and remove imo; one thing that is annoying about how it currently is is how intellisense works kinda badly with it - using a class would make it work better

3. The particular case of Color is interesting to me. After reading `utils/color.py`, I'm convinced that we should create our own class that inherits from `colour.Color`. This class would support alpha channels (which are currently handled haphazardly), produce all output as numpy arrays (so no more `np.array()` calls all over the codebase) and it may also contain pre-defined colors as classmethods/attributes as you suggest. What I'm saying is that we do not necessarily need to give colors the same treatment as other constants. But let's keep this for a future issue.

we could add a color parser which accepts multiple types of values, but yeah this is the wrong issue i guess

[leotrs]

So what I gather from this discussion is the following:

1. Use Enum for constant colors.
2. Use integers (specified in hex) for colors, instead of strings.
3. There should be a Color class that inherits from colour.Colour.

I think this issue pertains (1). The other two should be separate issues IMO.

[cobordism]

Can I put in a word for having colour be a synonym of color?
You have no idea how often this causes errors for people like me :)

[leotrs]

You mean alias every instance of color as colour?

[leotrs]

Also, point number 1 here is already implemented in #488. Please open new issues for points number 2 or 3 when necessary.

[cobordism]

You mean alias every instance of color as colour?

Is that not doable?
If not let's rename color to colour and annoy the other half for a change ;)

[leotrs]

Yes it should! I was just making sure I understood correctly. Also, can you please open a new issue for that? This one has ran its course.