manim icon indicating copy to clipboard operation
manim copied to clipboard

Making ManimColor an Immutable datatype

Open MartinXPN opened this issue 2 years ago • 14 comments

Description of bug / unexpected behavior

I'm trying to run manim with Python 3.11 (3.11.6 more specifically). Yet, I get a ValueError telling me that ManimColor class has a mutable default for a field color.

Expected behavior

It should work without errors.

How to reproduce the issue

Code for reproducing the problem

Place the following code in a file main.py

from manim import *

class CreateThumbnail(Scene):
    def construct(self):
        text = Text(
            'Some\nTitle', font_size=77
        ).center().shift(3 * LEFT).shift(2 * UP)
        self.add(text)

Run the following command to compile:

 python -m manim main.py CreateThumbnail -qm -v WARNING

Logs

Terminal output
...
...
    @dataclass
     ^^^^^^^^^
  File "/opt/homebrew/Cellar/[email protected]/3.11.6_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/dataclasses.py", line 1230, in dataclass
    return wrap(cls)
           ^^^^^^^^^
  File "/opt/homebrew/Cellar/[email protected]/3.11.6_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/dataclasses.py", line 1220, in wrap
    return _process_class(cls, init, repr, eq, order, unsafe_hash,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/[email protected]/3.11.6_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/dataclasses.py", line 958, in _process_class
    cls_fields.append(_get_field(cls, name, type, kw_only))
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/[email protected]/3.11.6_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/dataclasses.py", line 815, in _get_field
    raise ValueError(f'mutable default {type(f.default)} for field '
ValueError: mutable default <class 'manim.utils.color.core.ManimColor'> for field color is not allowed: use default_factory

System specifications

System Details
  • OS macOS 13.3
  • RAM: 64GB
  • Python version (python/py/python3 --version): 3.11
  • Installed modules (provide output from pip list): manim and numpy
FFMPEG

Output of ffmpeg -version:

ffmpeg version 6.0 Copyright (c) 2000-2023 the FFmpeg developers
built with Apple clang version 14.0.3 (clang-1403.0.22.14.1)
configuration: --prefix=/opt/homebrew/Cellar/ffmpeg/6.0_1 --enable-shared --enable-pthreads --enable-version3 --cc=clang --host-cflags= --host-ldflags= --enable-ffplay --enable-gnutls --enable-gpl --enable-libaom --enable-libaribb24 --enable-libbluray --enable-libdav1d --enable-libjxl --enable-libmp3lame --enable-libopus --enable-librav1e --enable-librist --enable-librubberband --enable-libsnappy --enable-libsrt --enable-libsvtav1 --enable-libtesseract --enable-libtheora --enable-libvidstab --enable-libvmaf --enable-libvorbis --enable-libvpx --enable-libwebp --enable-libx264 --enable-libx265 --enable-libxml2 --enable-libxvid --enable-lzma --enable-libfontconfig --enable-libfreetype --enable-frei0r --enable-libass --enable-libopencore-amrnb --enable-libopencore-amrwb --enable-libopenjpeg --enable-libspeex --enable-libsoxr --enable-libzmq --enable-libzimg --disable-libjack --disable-indev=jack --enable-videotoolbox --enable-audiotoolbox --enable-neon
libavutil      58.  2.100 / 58.  2.100
libavcodec     60.  3.100 / 60.  3.100
libavformat    60.  3.100 / 60.  3.100
libavdevice    60.  1.100 / 60.  1.100
libavfilter     9.  3.100 /  9.  3.100
libswscale      7.  1.100 /  7.  1.100
libswresample   4. 10.100 /  4. 10.100
libpostproc    57.  1.100 / 57.  1.100

MartinXPN avatar Nov 15 '23 07:11 MartinXPN

I was wondering if it would be possible to make the colors inside manim immutable? Otherwise, it's not possible to use them in dataclasses as default values.

MartinXPN avatar Nov 15 '23 07:11 MartinXPN

Weird. Just installed python 3.11.6 and installed manim and numpy in a fresh venv. FFMPEG version's also the same. Ran your code and everything rendered fine. Try a pip install -U manim and see if that fixes it.

MSameerAbbas avatar Nov 17 '23 16:11 MSameerAbbas

Sorry for the confusion. Seems like my provided code was not complete. Here is the complete example:

from manim import *
from dataclasses import dataclass


@dataclass
class Array:
    values: list[int | None]
    color: str = WHITE       # <- This causes an issue because Python now requires the default values to be non-mutable

class CreateThumbnail(Scene):
    def construct(self):
        text = Text(
            'Some\nTitle', font_size=77
        ).center().shift(3 * LEFT).shift(2 * UP)
        self.add(text)
    # ...

MartinXPN avatar Nov 17 '23 16:11 MartinXPN

How does this work as a solution?

from manim import *
from dataclasses import dataclass
from typing import ClassVar


@dataclass
class Array:
    values: list[int | None]
    color: ClassVar[ManimColor] = WHITE       # <- No longer raises a ValueError

class CreateThumbnail(Scene):
    def construct(self):
        text = Text(
            'Some\nTitle', font_size=77
        ).center().shift(3 * LEFT).shift(2 * UP)
        self.add(text)
    # ...```

MSameerAbbas avatar Nov 17 '23 18:11 MSameerAbbas

It doesn't. What if we need several different arrays? This should be an instance variable.

MartinXPN avatar Nov 17 '23 18:11 MartinXPN

color: str = WHITE

If you want to store it as a str, then the workaround would be to do so explicity: color: str = WHITE.to_hex().

Making ManimColor immutable is an interesting thought though. CC @MrDiver

behackl avatar Nov 17 '23 18:11 behackl

I would be happy to store it as another type as well. The problem is that it's currently mutable. So, the only way to have it in a dataclass is to have it with field(default_factory=lambda: WHITE). Which is not ideal especially when the colors can actually be immutable.

MartinXPN avatar Nov 17 '23 19:11 MartinXPN

I mean i kind of wrote it immutable so there shouldn't be methods that change the color in place if i didn't miss anything, so what would be the thing that signifies it's immutable?

MrDiver avatar Dec 01 '23 19:12 MrDiver

Maybe making ManimColor a frozen dataclass might help? Currently, the colors are of type ManimColor, which is a class; therefore, the values of all the colors are mutable.

MartinXPN avatar Dec 02 '23 11:12 MartinXPN

Let's see if I can manage to do this, I think the main problem will be getting the constructor to work. If you're on Discord it would be nice if you could join the dev-chat to maybe work out a solution that also works for different use-cases. Because i never really had the time to look at semantics of Dataclasses in that context.

MrDiver avatar Dec 02 '23 13:12 MrDiver

Just joined the Discord community. You should probably be able to accomplish everything that's done inside the current constructor with __post_init__().

MartinXPN avatar Dec 02 '23 13:12 MartinXPN

Hi @MrDiver are there any updates on this? Here are some thoughts that might be helpful:

What you wanna do is get rid of the constructor and place all the logic inside a __post_init__() That would ensure that the instance is initialized properly. You can create attributes inside it, etc.

I think from_xy should be a static method that returns a Color instance right? In that case, there shouldn't be any problems with the initialization

This might be helpful: https://chat.openai.com/share/511eb72a-c43f-442e-aa2a-8d48c9821c2e

This is the stack overflow question: https://stackoverflow.com/questions/53756788/how-to-set-the-value-of-dataclass-field-in-post-init-when-frozen-true

MartinXPN avatar Dec 16 '23 03:12 MartinXPN

Hey, yes i was still experimenting with that and trying to find edge cases where we might run into problems see the HSL color class. This can also be done immutable.

So it seems like this should be fine as a long term change.

MrDiver avatar Dec 16 '23 09:12 MrDiver

Another thought would be adding a __slots__.

JasonGrace2282 avatar Jun 23 '24 00:06 JasonGrace2282