# New colour enums- Try2

[kolibril13]

This is very similar to #342,
but with an approach like this:

class Colors(Enum):
    """A list of pre-defined colors."""

    dark_blue = "#236B8E"

DARK_BLUE = Colors.dark_blue.value

Where you get auto-completion and colours can be imported separately with from manim.utils.color import DARK_BLUE

can be tested with:

from manim import *

class Hello(Scene):
    def construct(self):
        dot = Dot().set_color(DARK_BLUE)
        self.add(dot)
        self.wait(1)

In case that this PR looks reasonable, further things to do are:

1. remove the colours from the config file
2. add the specfic imports all over the code base

[leotrs]

This LGTM, except I would also add an import statements in manim/init.py. for example if you add import utils.color as color in manim/init.py, then a user can import all colors as import manim.color as C. This is a bit cleaner than import manim.util.color as C.

[kolibril13]

This LGTM, except I would also add an import statements in manim/init.py. for example if you add import utils.color as color in manim/init.py, then a user can import all colors as import manim.color as C. This is a bit cleaner than import manim.util.color as C.

I guess you mean in __init__ the statement from .utils import color as color
Then, I can run it like this (with auto-completion!)

from manim import Scene, Dot
import manim.utils.color as C
class Hello(Scene):
    def construct(self):
        dot = Dot().set_color(C.DARK_BLUE)
        self.add(dot)
        self.wait(1)

But not import manim.color as C

Next step would be then, to go through all files and change the imports in a way, that only the colors needed are imported, like here https://github.com/ManimCommunity/manim/pull/342/files
Is there a way to do this automated?
EDIT: I now did this manually

[kolibril13]

And just saw that random_color() also has to be rewritten

[kolibril13]

I think this line does not have an effect right now.
import manim.color as C is not working, only
import manim.utils.color as C

[leotrs]

Can you please try from manim import color as C instead?

[kolibril13]

Oh, nice, this one is working

from manim import Scene,Dot
from manim import color as C
class Foo(Scene):
    def construct(self):
        dot = Dot().set_color(C.GREEN)
        self.add(dot)
        self.wait(1)

[leotrs]

@kolibril13 you seen to have run black on the whole codebase, as your PR now contains 58 files modified, including some that have nothing to do with colors. Can you please run black on only the files you have modified?

[kolibril13]

@kolibril13 you seen to have run black on the whole codebase, as your PR now contains 58 files modified, including some that have nothing to do with colors. Can you please run black on only the files you have modified?

Ok, now I reverted all the commits and ran black only on colors.py.
There are about 29 files that got one additional import line, and it is possible that black fails on them, but I don't know a good way of running black on only them. I think it is also not that important, as only the import line would be altered.

[leotrs]

Ok, now I reverted all the commits and ran black only on colors.py.
There are about 29 files that got one additional import line, and it is possible that black fails on them, but I don't know a good way of running black on only them. I think it is also not that important, as only the import line would be altered.

Thank you! And I agree, right now it's fine.

[leotrs]

Why not just import C? And use C.WHITE, etc.

[kolibril13]

also possible.
But I wanted to keep it consistent with the others, so I chose this way
Another option would be to say in this example from ...utils.color import *

[leotrs]

Almost there :)

[leotrs]

Oh real quick: I think there is a test_import somewhere, could you please add a quick test like

def test_import_color():
    import manim.util.color as C
    C.WHITE

@behackl Actually, could this be done through doctests?

[leotrs]

Also, can you please edit the changelog? Remember to add the fact that the colors have moved to an enum, as well as the fact that the user now has to set the random seed. Thanks :)

[behackl]

Oh real quick: I think there is a test_import somewhere, could you please add a quick test like

def test_import_color():
    import manim.util.color as C
    C.WHITE

@behackl Actually, could this be done through doctests?

Sure, just put something like

TESTS
=====

Check that <the corresponding issue> is fixed::

    >>> import manim.util.color as C
    >>> C.WHITE
    <put whatever output C.WHITE gives here>

[kolibril13]

the user now has to set the random seed

As this was also the case before, I think we don't have to add this to the changelog.

Oh real quick: I think there is a test_import somewhere, could you please add a quick test like

I scanned manimcm for this file. And there is nothing like test_import.
Should we add it?

[leotrs]

As this was also the case before, I think we don't have to add this to the changelog.

I'm pretty sure this is a difference with respect to 3b1b.

I scanned manimcm for this file. And there is nothing like test_import.

Woops, I must have been thinking about a new branch I have. Can you please add tests/test_color.py ?

[kolibril13]

I'm pretty sure this is a difference with respect to 3b1b.

I only added to the changelog "Added Colors have moved to an Enum".
Setting a random seed is not mandatory, it is only optional.
Normally, numpy does that automatically.
The behaviour is exactly the same as in manim3b1b, so I don't see a point in adding it into the changelog

[leotrs]

Ok, drop it.

[behackl]

I am a little late to this party, but whatever, just as a general FYI: if this would have been written as

.. code-block:: python
    
    >>> import manim.utils.color as C
    >>> C.WHITE
    '#FFFFFF'

then it would be a doctest, and these two lines would have been tested with every run of the pipeline.

[leotrs]

@kolibril13 let's do this :)

[kolibril13]

#496 ;)
Btw. does this mean, that the test can be removed?
All in all, I think we will have some redundancy when we have tests in the test folder and tests in the docs.

[behackl]

My rule of thumb for tests is: if it is easy to setup and maybe even illustrates something useful for users, then it can be a doctest.

Stuff that needs more setup is certainly better suited in dedicated test files. :-)