CONFIG dictionaries stuff

[huguesdevimeux]

We have discussed about the variable CONFIG previously on discord.
What should we do about it ?

I personally think we should modify at least a little bit of its architecture. it alters the readability a lot.
One problem with that would be that in changing CONFIG we would have to review the entire architecture of the code.

What do you think?

[leotrs]

I agree it's not the most elegant piece of the code. I'd love to hear some suggestions here. I have some experience with the configparser module from the standard library.

[leotrs]

A related issue is the constants.py file. Both this file and the CONFIG dict are basically ways of tracking hard-coded options. One nice-to-have would be to expose this options and allow the user to change or visualize them more easily. This would be achieved by the config files used by configparser.

[eulertour]

The CONFIG and constants.py issues can be handled separately, although we should do both. One of the confusing things about constants.py is that some of the variables defined there aren't actually constants (e.g. the media directories).

As for the CONFIG variables in each class I think it'd be cleaner to just add them as keyword arguments. We could leave the final **kwargs argument to accept additional config variables for backwards compatibility. For example Circle.__init__ here would be something like

def __init__(self, color=RED, close_new_points=True, anchors_span_full_range=False, **kwargs):
    self.color = color
    self.close_new_points = close_new_points
    self.anchors_span_full_range = anchors_span_full_range
    self.__dict__.update(kwargs)

[leotrs]

Is it always the case that doing self.CONFIG['key'] would be equivalent to doing self.key instead? for example, are the keys of the CONFIG dictionary iterated over?

Another thing to think about is: which of the things currently stored in CONFIG should be turned into properties.

[eulertour]

Is it always the case that doing self.CONFIG['key'] would be equivalent to doing self.key instead? for example, are the keys of the CONFIG dictionary iterated over?

Yes, but the code is pretty complicated.
https://github.com/ManimCommunity/manim/blob/master/manimlib/utils/config_ops.py#L23

[leotrs]

If that's the case, then I'd vote against converting each key of CONFIG into a member of the class. Because once they're just another member, there is no way of telling apart whether a member is supposed to be one that would go in CONFIG. Said another way, you can't iterate over only the members that are supposed to be CONFIGs without iterating over a bunch of members that are not.

[PgBiel]

If that's the case, then I'd vote against converting each key of CONFIG into a member of the class. Because once they're just another member, there is no way of telling apart whether a member is supposed to be one that would go in CONFIG. Said another way, you can't iterate over only the members that are supposed to be CONFIGs without iterating over a bunch of members that are not.

use __slots__ for this

[leotrs]

Oooooooooooooh good one

[yoshiask]

As someone who's most familiar with C#, the CONFIG is a big mess to me. There's no indication of which parameters are available and which ones aren't. kwargs has the same issue. This may just be a gripe with Python in general, but I think Manim is complicated enough without introducing unknown parameters.

[yoshiask]

I prefer @eulertour's proposal

As for the CONFIG variables in each class I think it'd be cleaner to just add them as keyword arguments. We could leave the final **kwargs argument to accept additional config variables for backwards compatibility.

[eulertour]

If that's the case, then I'd vote against converting each key of CONFIG into a member of the class. Because once they're just another member, there is no way of telling apart whether a member is supposed to be one that would go in CONFIG. Said another way, you can't iterate over only the members that are supposed to be CONFIGs without iterating over a bunch of members that are not.

@leotrs There is no difference between CONFIG variables and instance variables. They're one and the same.

[leotrs]

@leotrs There is no difference between CONFIG variables and instance variables. They're one and the same.

Well shit. Let's use kwargs and be done with it.

[huguesdevimeux]

There is a difference that is purely for readability purposes. As @yoshiask said, it put unknows parameters, which is not good in a quite complex library as Manim.
I’m completely in favor of @eulertour ‘point.

[PgBiel]

In my opinion, the usage of __slots__ makes it pretty clear to anyone who reads the code which attributes the class may have. So it will turn the code even more readable!

(Also note that __slots__ has memory advantages when many instances of a class are created, but this shouldn't be the main argument since it's pretty much irrelevant most of the time... but any improvement in this sense is welcome, of course)