Manim: Manually selecting a manim.cfg when NOT running from ClI

[huguesdevimeux]

Title.
It's not possible rn, as the .cfg files are chosen first when importing manim by a CLI flag.

This would be useful for tests, and for (maybe) a _potential_ migrating toward a manim that can be run from python files.

Pinging @leotrs

[leotrs]

Ok I've been thinking about this, and I'll make it my priority for now because it's holding up other things (#318, #317). If manim could be ran programmatically (and not only from the command line), testing would be much easier too. Also, I just realized that the configuration is actually being parsed (at least) twice, because the config._run_config() function is being called multiple times..... So here's my plan to do this. cc @Aathish04 @huguesdevimeux @naveen521kk @eulertour @PgBiel please share your thoughts.

I propose that we implement the following ways of rendering a scene using manim:

From the command line (already supported):

manim <file.py> SceneName -pl

From a python script:

import manim

class MyScene(manim.Scene):
    pass

# now, to render the scene, we do either of the following:
MyScene().render()
manim.render(MyScene)
manim.render(some_module)

• MyScene().render(). For this one to work, we need to implement a Scene.render method. Right now, a Scene is automatically rendered when it is constructed, but this can be easily changed by moving a few lines from Scene.__init__ to Scene.render, and calling render inside __main__.main() so that manim can still be called from the command line without any changes. This is an easy PR that we could implement ASAP (and, I think, regardless of the rest of this proposal). Note that this alone would simplify testing a good deal.

• manim.render(MyScene). For this one to work, we can implement a render method in __init__.py that just calls the newScene.render (see the previous paragraph).

• manim.render(some_module). When called in this way, this just searches any Scene classes in some_module and creates them and calls their render method. This is pretty much what the current __main__.main() does.

Now, the main problem (and going back to the present issue) is how to specify a config file when rendering programmatically, in any of the previous three ways. I suggest that we implement them as:

MyScene().render(config=None, config_file=None)
manim.render(scene_or_module, config=None, config_file=None)

The config argument is a dictionary that will _temporarily update the global config dict_. This means that if I do manim.render(MyScene, config={"background_color": WHITE}), then the global config will be used, but the "background_color" option will be overriden by WHITE. Right before returning, this function will restore the global config dict. Calling manim.render(MyScene) without a config argument just uses the global config dict untouched. Using the other argument, config_file, will read the specified file and add it as the highest priority to the cascading file system, and update the global config dict. The global config is again restored after the call returns. (We can perhaps cache the resulting overriden config dicts in case they are needed again in the future.)

Now, all of this pivots on the fact that executing import manim will correctly setup the global config dict. In particular, recall that import manim first executes __init__.py which in turn executes import config. So the order of events would be the following:

1. A script executes import manim, which executes manim/__init__.py. This can be a user scene script or our own manim/__main__.py.
2. In __init__.py, the first line of code is import config which executes config.py.
3. In config.py, _as much as possible_ of the config dict should be defined. In particular, all config files must be read and parsed. Another change I'm proposing is that config.py also parses the sections of the configuration corresponding to the logger, and exports a new dictionary called logger_config. All of config, camera_config, file_writer_config, and logger_config are defined and exported. NOTE the logger is NOT set here.
4. Back in __init__.py, the next line should be import logger. This executes logger.py, which will execute from config import logger_config, and use it to set the logger.
5. Here comes the split.
   
   
   
   • If the original script is a user script, then we are done and execution is returned to the user. They can then go about defining their Scene classes and ultimately calling manim.render(Scene, config=some_config_dict). If they do so, then the function manim.render will call config.udpate(some_config_dict), render the Scene, and then restore the original global config dict. Execution ends here. (Note this is also the case for tests or docs builds which require to import manim.)
   
   
   • If the original script was __main__ because manim has been called from the command line, __init__.py returns execution to __main__.main. Now, this function must deal with parsing command line flags and then updating the global config dict _permanently_. This can easily be done by e.g. manim.config.parse_cli(args). Note this will remove the problem we've had in the past where config.py had to determine whether it had been called from the command line. Once the CLI flags have been parsed and the config updated, __main__.main simply decides what to execute next, depending on the subcommand used. Thus, a big chunk of cfg_subcommands will be ported into __main__.py. In the event that the console command requires manim to render a scene, __main__.main can just call manim.render(module_passed_on_the_cli).
   
   

Note that in the process of making these changes, we will eliminate the hacky function _from_command_line, as well as eliminating the need of calling _run_config twice, and there will be even more consolidation of the whole mess that is the config system. (I'm 100% guilty of it being a mess.)

I hope this made some sense. I wanted to ran it by everyone before going into it because it will require a certain degree of surgery and being very careful.

Are there any objections? Did I miss anything?

[huguesdevimeux]

Some comments/questions :

• I think one function to render the scene is enough. Doing more is nonsense and will lead to confusion/complications. Furthermore, manim.render(some_module) will be useless IMO. If one wants to programmatically render a scene, one will specify which scene to render. When calling from CLI, you can't specify which scene to give to manim so I understand in what extent selecting the classes that can be rendered by manim is essential, but I can't see the point of such feature in a .py as people will have full control on it.

• We've been discussing about using decorators instead of that to render the scenes with @PgBiel and @kilacoda . Although I'm neutral on this point, maybe we should discuss here about which one to choose.

• Question, you are talking a lot about "temporarily update the config when calling from a python script" and ... I don't understand why this is necessary. Could you clarify ? (I don't think you did it in your message, if you did, sorry ^^)

• Last one : why is this related to #317 ? Am I missing something ?

[leotrs]

• I think one function to render the scene is enough. Doing more is nonsense and will lead to confusion/complications.

I agree, but here's a thing that I don't think I explained correctly. With Scene.render(), it will be awkward to do Scene.render(config=some_dict) and expect it to change the global config. Why should a Scene instance be expected to change the global config? That's why I think Scene.render() should not accept a config parameter and just use the global config dict, and that's also why manim.render(Scene) is more convenient. (I think this is different to what I said above, I apologize for the flip-flopping here.)

Furthermore, manim.render(some_module) will be useless IMO. If one wants to programmatically render a scene, one will specify which scene to render.

That's fair, let's scrap it.

• We've been discussing about using decorators instead of that to render the scenes with @PgBiel and @kilacoda . Although I'm neutral on this point, maybe we should discuss here about which one to choose.

Can you be more specific? I'm not married to either way of doing things (or at least not yet).

• Question, you are talking a lot about "temporarily update the config when calling from a python script" and ... I don't understand why this is necessary. Could you clarify ? (I don't think you did it in your message, if you did, sorry ^^)

This is so that you can render two different scenes with different configurations in the same session.

• Last one : why is this related to #317 ? Am I missing something ?

It's related to #317 because I wanted to add a test for the -n flag before fixing #317, to make sure that the fixes don't ruin something else. But then I realized that being able to change the config on the fly (or at least specify a config file programmatically, which is the topic of this issue) would've made that testing a lot easier.

[PgBiel]

• We've been discussing about using decorators instead of that to render the scenes with @PgBiel and @kilacoda . Although I'm neutral on this point, maybe we should discuss here about which one to choose.

Can you be more specific? I'm not married to either way of doing things (or at least not yet).

The idea is that you could specify which Scenes should be rendered in a file with a @render decorator. Something like:

class NotRendered(Scene):
   # ...

@render
class Rendered(Scene):
   # ...

@render
class AlsoRendered(Scene):
    # ...

manim.render(stuff=here, ...)

[leotrs]

I see. I don't think I like that idea that much because (i) the decorator seems to be doing little else than adding Scenes to a list, and (ii) there usually aren't that many scenes in a file so specifying them in manim.render(scenes_here, ...) wouldn't be too much of a hassle.

[leotrs]

Update: once #337 is done/merged/closed, we will be able to come back to this issue and easily close this one as well (hopefully). Note #337 has nothing to do with implementing manim.render or with the aforementioned decorators, so that's still up for discussion.

[leotrs]

Update: #340 has been merged and it introduces the tempconfig context manager, which should make it a lot easier to change _some_ config options in _some_ cases on the fly. This should reduce the number of cases where we need to litter the tests suite with custom manim.cfg files for each test. @huguesdevimeux

[huguesdevimeux]

Yes, but I suggest waiting to have a correct implementation of a scne-rendering from python files before touching this.

[leotrs]

I'm working on this. The tempconfig was the first step toward that goal

[leotrs]

In the course of implementing this I realized we don't have a clear plan for how to treat folder-wide config files when NOT running from the CLI. Or at least I don't know what the plan is :)

When manim is being imported (and NOT ran from the CLI), the library-wide file should 100% always be parsed. I think the user-wide config should also be parsed.

But what about a folder-wide config file, if it exists? If we also parse it by default, then writing import manim on the python interpreter will have different behaviors depending on where the interpreter was launched from. This is _super_ unorthodox, to the point that it makes me uneasy. I don't think this is the way to go.

Our alternatives:

1. Go with the unorthodox behavior and just use the folder-wide config.
2. Ignore it completely. The user should never be able to use the folder-wide config.
3. Do not parse it by default, but allow the user to do so if they want to. Concretely, we could add a function like so
   ```python
   $ python

import manim # the config is set using library-wide and user-wide files
manim.config.use_folder_config() # re-compute the config using the folder-wide file
```

Note that this impacts not only the (rare) times where a user imports manim from a python interpreter, but it will also impact tests and documentation builds. @huguesdevimeux how does this affect rendering from a script instead of from the CLI?

In any of these three cases, if manim is ran from the CLI, the folder-wide file will always be used.

[leotrs]

Of the above, I think that option number (3) is The Best(TM). However, it will necessarily require us to essentially make the config file twice. Once when manim is imported and then again when use_folder_config is called. I think this is _fine_ since these cases are actually very rare. When manim is called from the command line, the config will only be built once.

EDIT: actually now that I think about it, I think we should go with (1), because it will be weird to have to call user_folder_config every time that someone wants to render a scene from a script. (I was focusing on just using the interpreter and on tests, but rendering from script is something that we want to support, and it will not necessarily be rare). So my actual vote goes for (1) but with the caveat that every time we import manim, we should write a message on screen saying that "a folder-wide config file has been found" or some such.

[huguesdevimeux]

So my actual vote goes for (1) but with the caveat that every time we import manim, we should write a message on screen saying that "a folder-wide config file has been found" or some such.

Seems good to me.

@huguesdevimeux how does this affect rendering from a script instead of from the CLI?

I don't think it will affect anything, as long as we don't create a manim.cfg in the right place. When this idea will be concret, we will have to do a very small change in the test suite, but it's ok.

[leotrs]

Please see tempconfig and ManimConfig.digest_file.