Using Cleo instead of Argparse

[naveen521kk]

Currently, Manim uses Argparse for handling CLI arguments. It has been good but hard to maintain it, the currently introduced subcommands make it worse. Using something which was built for CLI management would help a lot of something like Cleo.

Cleo allows us to create beautiful and testable command-line commands. (Picked from docs) Where it uses docstrings to generate a command argument. (Bonus it allow autocompletion https://cleo.readthedocs.io/en/latest/introduction.html#autocompletion).

Many projects use this for CLI for example poetry.

[leotrs]

Yeah I've been thinking about ditching argparse too. Why Cleo and not Click?

[naveen521kk]

I didn't use any of the two. So your choice. For me, cleo looks more good.

[radiantly]

Hi, could I take up this issue?

However, it looks like click is more maintained compared to cleo so I think the switch should be made to click instead.

[leotrs]

@radiantly thanks so much for volunteering! This is a fairly major switch, so before you devote any time to it, I would like to have a discussion here about the pros and cons of each alternative. Why are we leaving argparse? Why should we take up any other library instead? Etc.

[cauebs]

Just to contribute to the discussion, you may want to take a look at typer, which wraps click :)

[leotrs]

@radiantly any updates on this? :)

[radiantly]

If the community comes to a decision, I can start working on the code.

A brief look at the options mentioned above:

• argparse - The codebase currently uses this, which as @naveen521kk mentions above, is good, but hard to maintain due to the lack of support for features like subcommands and lot of boilerplate code required.
• click - This is the most actively maintained library, if we were to switch, this would be first choice.
• cleo - Another library with a different philosophy when compared to click [1]. Not as actively maintained as click.
• typer - A wrapper around click that uses python annotations in an innovative way. The library is quite new and does not have 1.0 release yet, so I would suggest against using this for now.

[leotrs]

Thanks @radiantly. I do think that #620 does a good job at simplifying our argument parsing needs.

There is a trade-off here between how hard it is to maintain our own code vs how robust and supported the underlying library is. Right now, our code is hard to maintain, but there is TONS of support for argparse since it is part of stdlib. If we change to click (or other), our code may become cleaner, but we run the risk of going with something outside of stdlib (and adding yet another dependency).

@naveen521kk Can I ask you to please look at the argument parsing logic behind #620? Do you think that code is still too hard to maintain and we should incur in the risk of moving away from stdlib?

[naveen521kk]

I would suggest moving to another lib which is maintained would help us a lot in furture. For now, the current structure seems satisfying, but it would be difficult to add a new subcommand( I heard people working on addon system). Also, it will not hurt to add another dependency if it is more useful and keeps the logic simpler. Just because of an external dependency we should not make it hard to maintain also this looks like the same like that we picked rich.

[leotrs]

You are very convincing. I vote we go with the one that is the most supported, but let's do this after #620 is merged and stabilizes a bit.

Cc @ManimCommunity/core to hear from others.

[Aathish04]

I would highly recommend moving to another command line parsing library other than argparse, if only for the reason that implementing new subcommands is unnecessarily long winded. click would be my first choice, since it seems to be the most well maintained library (other than argparse or optparse) out of all our options.

[cobordism]

According to the all-dev call yesterday, whoever wants to rewrite CLI parsing has the go-ahead.
https://mensuel.framapad.org/p/manim-meeting
Simply assign yourself to this issue and the create a WIP PR to get going.

[leotrs]

From the cleo documentation:

As you may have already seen, Cleo uses the command docstring to determine the command definition. The docstring must be in the following form :

I am strongly against introducing functionality in the docstrings, so I would vote against using cleo. Also, click seems to have a wider audience (and thus, possibly, better support).

However, the person who works on this should have final say, so let's discuss once there's a PR 🙂

[jsonvillanueva]

I'm preemptively assigning myself this task as I'm currently learning Click for a different FOSS project. Feel free to take this up anytime within the next week @radiantly before I start the refactoring.

[jsonvillanueva]

After some time familiarizing myself with Click, I believe Click would run into issues translating the current argparse CLI positional argument handling.

For example:
manim file.py MyScene -pql

Currently, we manually intercept/filter commands and if the second argument matches one of the subcommands (i.e. {cfg,init,plugins}), we know that the argument is instead a subcommand -- not a positional arg like file.py. In Click, this paradigm doesn't exist out of the box as commands are either click.command(cls=None) (the default), or a click.group() ( i.e. click.command(cls=Group) ) which CAN'T be executed like a normal command and is instead used to nest subcommands which CAN be executed. There's some advanced patterns to consider that may allow Click to handle this usage -- I'd need to further experiment.

If the manim keyword is a Group containing cfg,init,plugins, and suppose for example, render, then we can translate argparse manim file.py ... to manim render file.py .... It seems using Click will involve some CLI command/option/argument restructuring/refactoring atm. Are we okay with potentially introducing a new CLI structure (say if I can't figure out this advanced pattern) so long as all current features remain accessible/present?

There is definitely a beauty in the simplistic notation of manim file.py ....

[leotrs]

@jsonvillanueva It sounds like this is a bit more complicated than expected. However, could you clarify the following: if you can't figure out a way of making manim file.py myscence -pql work, then what would be the alternative?

[jsonvillanueva]

@jsonvillanueva It sounds like this is a bit more complicated than expected. However, could you clarify the following: if you can't figure out a way of making manim file.py myscence -pql work, then what would be the alternative?

The alternative I currently see is to add the new subcommand, manim render which will be responsible for handling the required positional argument for file.py input (as well options such as Scene(s)/flags). This would make the current manim command only useful for manim --help to list subcommands/flags for the manim Group, manim --version to output the current manim version, and other things that don't explicitly require any subcommand to function. Also, the subcommands would automatically have their own help command (i.e. manim render --help, manim plugins --help, etc...)

[leotrs]

Could it be at all possible to support manim file.py myscene -pql?

Right now, we have some hacky ways of supporting sub-commands. If the move to click/cleo makes the handling of sub-commands clean, but we need some hacky ways of supporting manim file.py myscene -pql, then I think that'd be fine.

Unless others think that we should change to manim render altogether 🤔

[jsonvillanueva]

Could it be at all possible to support manim file.py myscene -pql?

Right now, we have some hacky ways of supporting sub-commands. If the move to click/cleo makes the handling of sub-commands clean, but we need some hacky ways of supporting manim file.py myscene -pql, then I think that'd be fine.

I've figured out how to hack/support using manim as both a command with non-required positional args and still use subcommands. Note, in argparse the manim command uses required positional args. It will take some time to finish this so it won't be in the upcoming release.

Unless others think that we should change to manim render altogether 🤔

@ManimCommunity/core --- I can't seem to use the tag. What do you guys think?

I think by design manim render is cleaner to maintain -- it's more as click intends, but also more verbose. I haven't implemented much yet, so design choices earlier on would be MUCH appreciated.

[leotrs]

On the one hand, manim render sounds unnecessary and/or obnoxious to me because it will be used the vast majority of times. I would much rather keep the usage without any subcommands.

On the other hand, the entire purpose of the config files is to make the CLI usage faster. So an experienced user would be using manim render file.py and let the config files do the rest. This doesn't seem too obnoxious to me.

So I'm of two minds about this, though I think I would like to still keep manim file.py if possible and not too hacky.

[cobordism]

On the one hand, manim render sounds unnecessary and/or obnoxious to me because it will be used the vast majority of times. I would much rather keep the usage without any subcommands.

See, I'm entirely on the other side here. To me, manim render sounds like just the right command to me when I want manim to render something. :)

[leotrs]

I agree! It's just that 90% of the time it will just be redundant I think. I guess what I'm saying is that "manim" should be synonymous with "manim render".

But whatever 🤷

[cobordism]

ah. ok.
So it could default to manim render whenever that works, and fail to manim --help when it does not... or sth like that if it is easy enough to implement.

[jsonvillanueva]

I agree, manim file.py ... should be a way to render files. It's nice and short.

So it could default to manim render whenever that works, and fail to manim --help when it does not... or sth like that if it is easy enough to implement.

Implementation aside, I think making manim default to manim render is a great approach. Currently, we don't have a notion of manim render; in argparse, manim takes full responsibility of that in its help page (which is really long). I think much of this length should be transferred to manim render --help, and manim --help should have some notice that says it defaults to manim render. Besides subcommands (render, plugins, cfg) and the --help/--version options, should any other options be nested directly under manim?

By the way the package click-default-group handles this nicer than I would've in manually recreating our current approach (i.e. manim is the render command). Also, there's other click-contrib packages that may be of interest including:

• click-plugins
• click-configfile
• click-log
• and more...