ManimColor: Adding documentation and examples for creating colors

[ubaldot]

Time to work with colours! :D

BG_rgb = (40, 85, 120)
hsv = ManimColor().from_rgb(color).to_hsv()

will return TypeError: ManimColor.__init__() missing 1 required positional argument: 'value', whereas the docs say

class ManimColor(value, alpha=1.0)

...
 PARAMETERS:
-  value (ParsableManimColor | None) – Some representation of a color (e.g., a string or a suitable tuple).

Is that None a "docs' bug"?

Questions:

1. How shall I create a hsv ManimColor from a rgb/hsv tuple?
2. Once I have some sort of hsv = ManimColor(..., wouldn't be nicer to access attributes e.g. with hsv.hue, hsv.saturation‚ hsv.value rather than using the .to_hsv()method?
3. I noticed that the hsl color space has been removed, is there any special reason?
4. Would you mind to add some examples in the docs so the users could better understand how to create their colours?

Keep up the good work !

[MrDiver]

That None is indeed not a bug and currently just for not needing to rewrite the whole library it just returns BLACK if you give it None

The reason for you needing to convert to hsv before you can access the values is that the internal representation is now in rgba of floats which is more in line of how the color is handled in rendering.

If HSL is missing i think i just forgot that, we just need to add another conversion function

The problem with giving attributes for accessing colors in different formats is that you will cause a wave of conversions for every time you do that and it slowed down manim a lot when handling a lot of colors that change.

New examples seem like a good idea but it would probably just be showing every method once that is already documented on the ManimColor page, if you have any ideas on how to do that in a useful way i would be happy to do something about it.

In general if you want to create colors from hsv then you just use the static function from_hsv that gives some more work to the user but avoids a lot of unnecessary copying and conversion for no other reason than the user used the color.

[MrDiver]

And to answer your original question

>>> hsv = ManimColor.from_rgb(None).to_hsv()
>>> hsv
(0.0, 0.0, 0.0)
>>> hsv = ManimColor(None)
>>> hsv
ManimColor('#000000') 

None doesn't mean Nothing

[ubaldot]

Ah ok, I got it. So one must consider a class ref, not a class instance, e.g. this:

>>> hsv = ManimColor.from_rgb(None).to_hsv()

instead of this:

>>> hsv = ManimColor().from_rgb(None).to_hsv()

That does not appear anywhere in the docs, and a user is not supposed to dig into the source code.

Overall, I think that the documentation shall be updated because it is very confusing if it is intended to be for the final user. For example, the fact that ManimColor shall be referred as a class ref and not as a class instance is paramount to be documented, whereas info like the following:

This is done in order to reduce the amount of color inconsitencies by constantly casting between integers and floats which introduces errors.

has of little - if none - utility for the users.

Furthermore, the following, other than being poorly documented, is also wrong:

ManimColor, int, str, RGB_Tuple_Int, RGB_Tuple_Float, RGBA_Tuple_Int, RGBA_Tuple_Float, RGB_Array_Int, RGB_Array_Float, RGBA_Array_Int, RGBA_Array_Float

For example, ManimColor("BlueMarine") return a Value error, so str is not a correct datatype, but it should be an element in a set of strings. And these issues/errors are only those who appeared in my eyes at first glance and without going any deeper.

[behackl]

For example, the fact that ManimColor shall be referred as a class ref and not as a class instance is paramount to be documented

These methods are all listed as class methods, the information is there. Oneline examples of how to call them would be nice though. Suggestions for how to make the distinction between class and instance methods more visible are welcome too.

[ubaldot]

I would do the following way:

1. Add a Warning that differently from he wide majority of users' Manim classes, objects are instantiated through classmethods, for example, with:

>>> hsv = ManimColor.from_rgb(0.3, 0.4, 0.8).to_hsv()

and therefore, calls like hsv = ManimColor(xyz) won't work.

2. Remove all things that distract the user, but are useful for developers, such as

This is done in order to reduce the amount of color inconsitencies by constantly casting between integers and floats which introduces errors.

3. Avoid reporting internal methods in the user docs (those who start with _)

4. Proof read the docs from possibly some users (not developers).

5. Clean up the overall page (for example classmethod parse is reported twice).

[behackl]

1. Add a Warning that differently from he wide majority of users' Manim classes, objects are instantiated through classmethods [...] and therefore, calls like hsv = ManimColor(xyz) won't work.

This is not the case. The class methods (which examples could be added to) are an additional interface for creating instances. Based on RGB values, you can still create an object in the conventional way via ManimColor((R, G, B)). You do not need to use the static from_rgb method.

2. Remove all things that distract the user, but are useful for developers, such as [...]

I am not sure I agree with the complete removal; there is information relevant for devs that needs to be put somewhere too; docstrings are not purely user-facing features.

3. Avoid reporting internal methods in the user docs (those who start with _)

I agree, that is not difficult to change. We should investigate whether there was a particular reason why this was enabled.

4. Proof read the docs from possibly some users (not developers).

Contributions and improvements to the documentation are welcome!

5. Clean up the overall page (for example classmethod parse is reported twice).

The signature is listed twice because there are two different return types, Sphinx generates it like that. Not sure we have much more control about how exactly this is displayed.

[ubaldot]

I am just giving a feedback as end-user if that count as contribution. :-)

I had really hard time in understanding this page compared to many other pages in the docs. And I failed. And the more I read, the more confused I became. And I do know a bit of python, I try to wear the shoes of a “standard user”. But perhaps I didn’t understand what is the Manim target audience? That could be clarified somewhere I think. For example, are Manim users expected to know the difference between methods and classmethods?

Also the usage could be clarified. Is it good to use in a production environment? Or is it a sort of experimental open lab to practice coding? Note that there is no right or wrong answer, but clarifying these things would set the correct level of expectation from users, that will react accordingly to the gap between reality and expectation so to say. That is, if the aim is production I will adopt a certain line since I would have some expectation, if it is an open lab for practicing I would approach it differently.

I think it is about time to write down Vision and Mission? :-)

Bottom line: Consider this as a end-user feedback and nothing more than that. :-)

[MrDiver]

So i would now word it a bit different because saying something like it has no utility for the user is really trashing the whole picture why this change was made in the first place.

Yes it has utility for the user even if you do not see it directly, render times are reduced, many errors have been solved, color shifting due to the constant conversions has been completely removed as a problem.

So please do not go around and say something like it has no utility for the user if you're seeing a tiny part of it.

Next point is, yes it is kind of an expectation that you read the documentation and if you do not understand something then you're able to look up why it may not work or ask on discord. Because personally i think the documentation is pretty clear on which functions are class functions and which are not.

And if you say things like time to write down a mission please join us in the discord and become a dev and actually look at what is done behind the scenes, because there are clear goals for the library

And lastly i would also suggest to you that you first think about multiple stand points for which things might be used because just having one perspective really shifts the reality of the situation, so keep answers short and useful and provide use cases and information that is relevant to the dev process instead of using exaggerations in every sentence.

Thanks for your suggestion

[ubaldot]

@behackl

1. Correct. Then, what is the purpose of .from_rgb()? Given 4., I would document something like the following:

If you have a RGB triple, then you can create a ManimColor instance with e.g.

mycolor = ManimColor((0.1, 0.4, 0,3)) # assuming RBG values are floats between 0 and 1
mycolor = ManimColor((125, 201, 98)) # assuming RGB values are integers between 0 and 255

However, if you have colours expressed in a different color-space (for example HSV), then you can create ManimColor instances through the so called _classmethods_, for example: 

mycolor = ManimColor.from_hsv((0.3, 0.6, 0.8)) # assuming HSV values are floats between 0 and 1
mycolor = ManimColor.from_hsv((122, 98, 200)) # assuming HSV values are integers between 0 and 255

2. Well, a rule of thumb (which I really support) is to document dev info in comments and users info in docstrings.

I understand now issues related to the remaining points. Thanks.