Skip to content

Introduce seed in random_color method to produce colors deterministically #4265

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

ishu9bansal
Copy link

Overview: What does this pull request change?

  • Introduces a new class RandomColorGenerator, which enables deterministic generation of colors from Manim's color palette using an optional seed.
  • Refactors the random_color() function to use this class internally, preserving existing behavior and output style.

Motivation and Explanation: Why and how do your changes improve the library?

The existing random_color() function uses Python’s global random module, which makes the results non-deterministic and unsuitable for use cases like tests or reproducible animations.

This PR introduces RandomColorGenerator, a utility that allows users to generate a repeatable sequence of random Manim colors by initializing it with a specific seed. The sequence is generated statefully—each call to .next() returns the next color in that seeded sequence. Here’s an example:

>>> from manim.utils.color.core import RandomColorGenerator
>>> rnd = RandomColorGenerator(42)
>>> rnd.next(), rnd.next()
(ManimColor('#ECE7E2'), ManimColor('#BBBBBB'))

>>> rnd2 = RandomColorGenerator(42)
>>> rnd2.next(), rnd2.next()
(ManimColor('#ECE7E2'), ManimColor('#BBBBBB'))  # same as above

Meanwhile, the random_color() function continues to behave the same:

>>> from manim import random_color
>>> random_color()
ManimColor('#9A72AC')

This ensures full backward compatibility, with zero impact on existing code using random_color().

Further Information and Comments

Based on the discussion in the related issue, this design addresses two key requirements:

  • Provide a deterministic way to generate random colors for testing or consistent rendering.
  • Do not introduce breaking changes to the existing random_color() API, which is widely used.

Importantly, we do not add a seed parameter to random_color() to avoid confusion around stateless vs stateful behavior. Instead, the RandomColorGenerator class provides an explicit, opt-in mechanism for seeded, stateful random color generation—making the intended behavior clear to users.

This separation keeps the API clean while offering more control to advanced users who need it.

Reviewer Checklist

  • The PR title is descriptive enough for the changelog, and the PR is labeled correctly
  • If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
  • If applicable: newly added functions and classes are tested

@ishu9bansal
Copy link
Author

Referencing issue #1671

I’m open to further discussion to fine-tune the implementation. For example, we could consider using a singleton instance in the default (non-seeded) case instead of creating a new object each time.

If the overall approach looks good, I’m happy to iterate further and refine the PR for production readiness.

Copy link
Member

@behackl behackl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a nice proposal, and a good resolution for this over-due issue. This approach would still work (or at least, could be made to work) in case we ever decide to introduce a module-global seed (ManimConfig.random_seed ...).

Could you please:

  • double check that the line in the docstring, L1515 is not too long and break it if otherwise,
  • add a PARAMETERS-section to the class docstring where the seed argument of the initializer is documented,
  • and add the fixed-seed example from the description either as a unit test (together with the other color module tests) or as a doctest in an EXAMPLES section of the docstring? (I have a slight preference for the latter, as it simultaneously documents the usage of the generator class.)

Thank you for your contribution!

@ishu9bansal
Copy link
Author

Great!
Let me make the changes you suggested. @behackl
Meanwhile, please assign this issue to me.

@uwezi
Copy link
Contributor

uwezi commented Jun 4, 2025

May I add a little suggestion: I now often use np.random.choice() to get a random color from a given list of colors, since not all colors usually fit to the theme of a scene, particularly also if the background color is included in the draw.

Could you consider to add an additional, optional parameter to the __init__ which sets a list of colors instead of manim_colors._all_manim_colors?

One additional question: since numpy is already present in Manim, is there any reason to use random instead of numpy.random?

@ishu9bansal
Copy link
Author

ishu9bansal commented Jun 5, 2025

Could you consider to add an additional, optional parameter to the __init__ which sets a list of colors instead of manim_colors._all_manim_colors?

@uwezi
Sure, we can take the list of colors as the given pallet in init params, and default it to _all_manim_colors.
I can add this as well.

Update:
Done ✅

@ishu9bansal
Copy link
Author

@behackl please review and suggest anything else that needs to be changed.

) -> None:
self.choice = random.choice if seed is None else random.Random(seed).choice

from manim.utils.color.manim_colors import _all_manim_colors

Check notice

Code scanning / CodeQL

Cyclic import Note

Import of module
manim.utils.color.manim_colors
begins an import cycle.
Copy link
Member

@behackl behackl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I've left a hand full of suggestions to improve how the docstrings are rendered, please take a look.

I'll also push one additional commit in a second to fix the rendering of the code snippets.

Other than that, this seems good to go for me. Let me know once you've checked my suggested changes!

Comment on lines +1515 to +1516
"""
A generator for producing random colors from a given list of Manim colors,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""
A generator for producing random colors from a given list of Manim colors,
"""A generator for producing random colors from a given list of Manim colors,

optionally in a reproducible sequence using a seed value.
When initialized with a specific seed, this class produces a deterministic
sequence of `ManimColor` instances. If no seed is provided, the selection is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sequence of `ManimColor` instances. If no seed is provided, the selection is
sequence of :class:`.ManimColor` instances. If no seed is provided, the selection is

Parameters
----------
seed : int | None, optional
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type annoations are taken from the actual type hints, no need to add them twice

Suggested change
seed : int | None, optional
seed

A seed value to initialize the internal random number generator.
If None (default), colors are chosen using the global random state.
sample_colors : list[ManimColor], optional
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sample_colors : list[ManimColor], optional
sample_colors

----------
seed : int | None, optional
A seed value to initialize the internal random number generator.
If None (default), colors are chosen using the global random state.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If None (default), colors are chosen using the global random state.
If ``None`` (the default), colors are chosen using the global random state.

If None (default), colors are chosen using the global random state.
sample_colors : list[ManimColor], optional
A custom list of Manim colors to sample from. Defaults to the full Manim color palette.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
A custom list of Manim colors to sample from. Defaults to the full Manim color palette.
A custom list of Manim colors to sample from. Defaults to the full Manim
color palette.

Examples
--------
Without a seed (non-deterministic):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual formatting for the example blocks need to be slightly adapted to make the documentation render correctly. I'll push a commit to take care of it. 👍

Comment on lines +1584 to +1585
"""
Returns the next color from the configured color list.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""
Returns the next color from the configured color list.
"""Return the next color from the configured color list.

@behackl
Copy link
Member

behackl commented Jun 7, 2025

One additional thing I just noticed: there is a warning in the docstring of the random_color function concerning its performance -- I'm not sure there is a (still) good reason for it to be around. Perhaps it should be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

random_color() and random_bright_color() should have a parameter random_seed
3 participants