Replace the global config dict with new class ManimConfig

[leotrs]

Motivation

Short version: the config system is Bad. This PR makes it Less Bad by introducing a new dict-like class ManimConfig. Instances of this class behave pretty much like the previous global config dict, but maintain internal consistency, among other features. Long version: see #337.

List of Changes

The diff is long. Here I explain the changes file by file. Hopefully this will make reviewing easier. Note most of the changes are due to the removal of file_writer_config.

• Take the logger logic from the file manim/config/utils.py into a new file manim/config/logger.py. This also explains the diff in tests/control_data/logs_data/BasicSceneLoggingTest.txt.
• This PR removes the global dicts file_writer_config, camera_config and config, and replaces them with a (still global) instance of the ManimConfig class. This global is also called config. This accounts for the diff in manim/__main__.py, manim/camera/camera.py, manim/config/__init__.py, manim/grpc/impl/frame_server_impl.py , manim/mobject/mobject.py, manim/mobject/svg/text_mobject.py, manim/utils/module_ops.py, tests/utils/GraphicalUnitTester.py, tests/test_copy.py, tests/test_container.py, tests/helpers/graphical_units.py, and manim/utils/tex_file_writing.py.
• Add the config options video_dir, tex_dir, text_dir, images_dir to manim/config/default.cfg. These DO NOT use the placeholder syntax used by configparser. Instead, they use f-string syntax, as used by ManimConfig._get_dir().
• The file manim/config/main_utils.py has two modifications: the init_dirs function has been moved to manim/config/utils.py and the function update_config_with_cli has disappeared and its logic moved inside ManimConfig.digest_args().
• manim/constants.py: we have changed the default resolution from "production" to "high" (I believe either @kolibril13 or @huguesdevimeux did this) so I had to update one line in this file.
• added a test to test the -s flag to tests/test_scene_rendering/test_cli_flags.py
• remove the file tests/test_scene_rendering/test_caching_relalted.py, which had the same content as tests/test_scene_rendering/test_caching_related.py (note the typo). This was already removed before, but I don't know how it snuck into my branch 🤷
• The files tests/test_logging/test_logging.py, tests/test_scene_rendering/test_caching_related.py, tests/test_scene_rendering/conftest.py, and tests/test_cli_flags.py each have a one-line quality of live improvement.
• tests/test_config.py has been modified because ManimConfig no longer allows adding/updating keys that did not previously exist.
• The diff in manim/scene/scene_file_writer.py is explained by the removal of scene_file_writer EXCEPT for the new logic in lilne 128. This change in logic now checks whether a file exists before using it (which makes sense and has nothing to do with the ManimConfig class!)
• The diff in manim/scene/scene.py is explained by the removal of scene_file_writer EXCEPT for the change in logic in lines 100-102 (some child objects are now made to use the global config objects instead of passing around things like **camera_config) and the logic in line 128, which now correctly restores the previous state of skip_animations (before it was always just set to False, which makes no sense).
• manim/config/utils.py - this is the big one. The init_dirs() function comes from manim/config/main_utils.py, and the parser logic has moved to manim/config/parser.py. The big change: it now contains the definition of the two main classes ManimConfig and ManimFrame. More on these later.

Explanation for Changes

Ok the main idea of ManimConfig is the following. It behaves like a dict but it keeps track of all the logic between different config options. To behave like a dict, it implements some dunder methods such as __iter__, __len__, __contains__, __getitem__, __setitem__. However, for documentation purposes, each config option is also an attribute. For example the config option write_to_movie can be accessed as config["write_to_movie"] or as config.write_to_movie. The former is backwards compatible, while the latter allows for easy documentation and auto-completion. To implement this behavior, there is a private attribute self._d which is an actual dictionary which contains the config options as keys. The user should never access it directly. Most of the get/set code touches self._d in one way or another. Furthermore, ManimConfig does not allow to add new keys or remove existing keys (see __delitem__ and __delattr__).

ManimConfig can only be initialized by passing a ConfigParser instance. This will usually contain the options set in manim/config/default.cfg, as well as any other config files such as the folder-wide or the user-wide. The global config is instantiated on line 25 of manim/config/__init__.py with the config parser object returned by manim/config/utils.py::make_config_parser. During initialization, the function digest_parser() is called, which consumes the parser. After this, all config files have been parsed and digested into the global config, and manim is ready to go.

Now, if manim is being called from the command line, then manim/__main__.py will call config.digest_args(args). This will take the parsed sys.argv args and update the necessary config options. Note that the options from the config files and from the CLI flags are parsed at different times. Because of this, there is no longer any need of hacky things such as trying to guess whether manim was being used from the command line or not. This is a huge gain in readability I think.

I have left placeholder methods ManimConfig.digest_dict and ManimConfig.digest_file. The latter will eventually allow to read a configuration file on the fly. This feature was requested by @ManimCommunity/tests and will simplify testing a lot.

Finally, the rest of the code of ManimConfig is the definition of a ton of properties. Each config option is now a property. In this way, we have complete control of what happens when a config option is read or set. In this way, we keep internal consistency of all options. For example, see ManimConfig.tex_template_file or ManimConfig.dry_run. Look also at ManimConfig.quality which now allows to change the width, height, and frame rate with a single line of code config["quality"] = "low_quality" (@eulertour recently requested a set_quality method 😉 ). The rest of the properties are mostly boilerplate. I chose to use the syntax config_opt = property(...) because it reduces boilerplate and wasted space (though I am not married to this). There are a few helpers that help with this boilerplate (_set_from_list, _set_boolean, _set_str, _set_between, _set_pos_number).

The ManimConfig class is meant to contain all the configuration in one place, a single source of truth, and it replaces file_writer_config, and most of config, and is meant to be used mostly internally. HOWEVER, I have also added a new class, ManimFrame, which is meant to contain the config options that are related to frame/video geometry and are meant to be used mostly by the user. The global frame is called frame and is instantiated in line 26 of manim/config/__init__.py and depends intimately on the global ManimConfig object. In short, ManimFrame just reflects a bunch of config options set in ManimConfig (see bottom of the file manim/config/utils.py) but a ManimFrame is completely immutable. My intention is to allow the user to do something like from manim import frame as F and then do something like F.pixel_width or F.top. (This is something @kolibril13 has been asking for for a while!) However, I'm not married to the current implementation of ManimFrame and that is subject to change. Please let's focus right now on ManimConfig first!

That's all folks. I've done my best to explain the inner workings of this beast. Please let me know if I missed anything.

Testing Status

Passes locally. Added a new test for the -s flag.

Further Comments

Right now the ManimFrame class is implemented but never used. I wanted to reduce the size of this PR as much as possible before actually using it. If you have general comments on it, please leave them here. If you have implementation comments, let's discuss them in the next PR.

Once the review of this stabilizes a bit, I will write the changelog right before merging. I will add extensive documentation in a separate PR.

I still want to add a test for --disable_caching before merging this, but haven't figured out how. Help please @ManimCommunity/tests.

Acknowledgement

• I have read the Contributing Guidelines

[eulertour]

Everything here looks good to me, it's just a matter of addressing the problems with the merge and docs.

[leotrs]

Just pushed a new commit that fixes the -s flag when the scene contains no animations. Thanks @kolibril13 for reporting. (It also removes a couple of repeated keys.)

@ManimCommunity/tests could you please double-check the test that was added inhttps://github.com/ManimCommunity/manim/pull/620/commits/e74289e57955cf160239a6337370f7a8ef4a2d54? I'm not sure if there's a better way of doing this.

[leotrs]

Just a request, that is not implemented (or didn't I noticed) :
A method to revert manim config to the previous state.
I don't know if iy would be particularly useful, but who knows.

tempconfig exists! It even has a couple of tests already.

This is typically the type of thing that needs ton of tests, if we don't want it to well, break at every PR.
Could you add some please?

Yeah, I want to add a few tests for some more CLI flags, at least. Do you have suggestions on what else I should be testing?

[leotrs]

Update: tests pass locally and docs build.

However, there are a few tests that break only in python3.6. I'll investigate.

[leotrs]

All tests pass! Now I'll add some more tests as requested by @huguesdevimeux

[leotrs]

(Hopefully) last update: tests pass, docs build. Added a few more tests and cleaned up some unnecessary code.

@eulertour I believe there are no tests for the js renderer so if you want to test that out before merging, that'd be great.

Otherwise, I move we merge this ASAP because rebasing is killing me. I will write documentation in a separate PR.

[behackl]

This looks great; amazing job! I love how readable especially arg parsing suddenly is. 🚀

I only have two issues/comments, but they can both be settled in follow-up PRs.

[behackl]

Detailed documentation for module-level functions is currently not rendered it seems (the oneline-summary is rendered in the list of functions, but documentation for parameters and notes etc. are not) -- we should take care of this in a separate PR, though.

[leotrs]

@eulertour do you want to test JS renderer stuff before I merge and inevitably break everything?

[eulertour]

I'm actually fine with you merging it first, since after making the CairoRenderer the JS renderer will have to be reworked anyway