Rich Command line output colour customization

[naveen521kk]

In this PR I have changed the logging and also added an option on customizing colored output using a rich.cfg file. SEE the screenshot below.

Below is how the configuration works out

[eulertour]

Instead of creating another .cfg file we should wait until #98 is merged and add a section to that one.

[naveen521kk]

@eulertour Can you tell me what should I change so that it is compatible with it! Like should I create a place in manim.cfg or something like that as I don't know what you have planned in #98.

[eulertour]

Essentially create a [color] section or something similar in the file, and store the defaults there instead of in rich.cfg. The code or the docs here will be the best references.

[naveen521kk]

I will work on it and make it compatible! Also, I need to ask does this work in other OS, 'cause I have tested this only in Windows. Maybe others could check it out 'cause it should not cause an error when users use it.

[naveen521kk]

Also, @eulertour manim.cfg will have to be placed in the current working by the user right?

[leotrs]

Also, @eulertour is manim.cfg to be placed in the current working by the user right?

Yes.

[naveen521kk]

I think 🤔 I did it. Could you please whether this work.
Type in python -m manim.logger to open the colour configuration. It will create manim.cfg. Also, where can I write some documentation for it?

[PgBiel]

I think 🤔 I did it. Could you please whether this work.
Type in python -m manim.logger to open the colour configuration. It will create manim.cfg. Also, where can I write some documentation for it?

manim.cfg doesn't exist yet. It is being implemented in #98. Once that is merged, you will have to adapt your code to the new system

[naveen521kk]

I think thinking I did it. Could you please whether this work.
Type in python -m manim.logger to open the colour configuration. It will create manim.cfg. Also, where can I write some documentation for it?

manim.cfg doesn't exist yet. It is being implemented in #98. Once that is merged, you will have to adapt your code to the new system

I had an idea of how manim.cfg would and have just implemented what I thought it would be. I will edit it accordingly once #98 is merged. Also, where should I write documentation about this feature?

[naveen521kk]

Now #98 is merged what should I do for it to be merged?

[leotrs]

Hi @naveen521kk. Thanks for following up. I have not read the code for this PR, but the steps to follow now are essentially the following. Let' say you want to change the code of the output (I don't know how to do this, this is purely hypothetical, but I hope these steps clarify the problem.)

1. First, you need to add a new section to the default.cfg file inside manim/. You can call it whatever you want, for example [rich].
2. Under that section, you can define whatever config variables you want, for example rich_output_color.
3. Inside config.py, you need to modify _parse_config. The value of rich_output_color that was set in default.cfg will be available to you within that function as config_parser['rich']['rich_output_color']. Please note that all values of config_parser are strings. If you need other types, you must convert them from string (see documentation here). Once you have read (and/or converted, modified, or parsed) the value of config_parser['rich']['rich_output_color'], you need to store it in config. Please use the same key, but without the section name. For example config['rich_output_color'] = do_something(config_parser['rich']['rich_output_color']).
4. You must modify the logger configuration using the new config variables. Let's assume this is done in logger.py. In that file, you must import the configuration from .config import config and then use the config dictionary to set the logger configuration. For example logging.basicConfig(color=config['rich_output_color'], ...).

All of the above are of course examples and you will have to change the things that need changing. In summary, you need to make the following changes.

default.cfg

...
[rich]
rich_output_color = <some_color>
...

config.py

def _parse_config(config_parser, args):
    ...
    config['rich_output_color'] = do_something(config_parser['rich']['rich_output_color'])
    ...

logger.py

from .config import config
`logging.basicConfig(color=config['rich_output_color'], ...)`

Please do not modify other parts of default.cfg or config.py that have nothing to do with the config options you are adding. Thanks! Hope that helps!

[naveen521kk]

So @leotrs I tried for the new config parser and #98 I found a problem. I am trying out that rich coloured output PR. I tried

python -m manim.logger

To execute the __main__.py file in logger directory which worked before this update. But now whatever I do, argparse runs up. Is this indented? If yes how can I really execute that file? Also,

python -m manim.anything

Just execute the argparse ever if there is no submodule. Looks like this need to be checked.

[naveen521kk]

I have changed the submodule to logger.py as @Aathish04 told in Discord. Also, I just noticed that config.py depends on logger.py. So, I have created a new config parser. If you have a better idea than that please tell. It is ready for review.

[Aathish04]

Changes Requested (a summary):

1. Usage of configparser.config and/or configparser.file_writer_config instead of implementing another config parser.
2. Renaming of command to run the configuration generator.
3. Removal of some commented out code.

[Aathish04]

Changes Requested:

• Turn a not-so-critical error into a warning
• Make the title indicate that this is a general purpose tool.
• Edit around a link that isn't properly displayed.

@naveen521kk Pull Request for these changes: naveen521kk#6
EDIT : For the sake of convenience, I've committed the changes directly.

[Aathish04]

Just some formatting, string and code clarity.

[Aathish04]

Approved.

Mentioning @leotrs for his review and thoughts.

Please don't merge until @leotrs has given his review.

[leotrs]

@naveen521kk Good job on the latest commits! One standing question is that I'd love to merge a test along with this as well. @huguesdevimeux ideas?

[huguesdevimeux]

@naveen521kk Good job on the latest commits! One standing question is that I'd love to merge a test along with this as well. @huguesdevimeux ideas?

Well, I don’t know at all.
I heard that there is/will be a PR to save logs. Maybe we will be able to build a test when this is merged.
(But To be very frank, I don’t think we should wait to have a test before merging this, as this is not crucial).

[leotrs]

(But To be very frank, I don’t think we should wait to have a test before merging this, as this is not crucial).

Hmm fair, let's talk about tests in #201 then