Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring the current cli and config of manim to use a combined system for specification #3078

Closed
wants to merge 2 commits into from

Conversation

MrDiver
Copy link
Collaborator

@MrDiver MrDiver commented Dec 18, 2022

Overview: What does this pull request change?

This PR aims to refactor the current state of the cli and config options because there is a lot of duplication spread throughout manim which makes it really difficult to maintain the cli and the config options.

My goal is to unify both systems into one specification which can easily be changed to add new parameters and still provide good documentation or parsing abilities either in the cli or through config files.

@MrDiver MrDiver added enhancement Additions and improvements in general maintenance refactoring, typos, removing clutter/dead code, and other code quality improvements labels Dec 18, 2022
@MrDiver MrDiver added this to the v0.18.0 milestone Dec 18, 2022
@MrDiver MrDiver added the breaking changes This PR introduces breaking changes label Dec 18, 2022
@MrDiver MrDiver added the help wanted We would appreciate help on this issue/PR label Jan 2, 2023
@MrDiver MrDiver changed the base branch from main to experimental January 3, 2023 11:53
@behackl
Copy link
Member

behackl commented Aug 11, 2023

We just sat down together (well, sort of togehter) and discussed the current state of CLI config options. Here are some (rather informal) comments

--custom_folders ???????????

--tex_template: MrDiver uses that, do not delete

--enable_gui: documentation?! does it still do something?

--notify_outdated_version / --silent: why are these two aliases? check should be smarter in case of no internet

--enable_wireframe: resolution does not scale, trash. delete. document using renderdoc instead.

--force_window: alias for -p for OpenGL? delete

--dry_run: is it *really* a dry run?

-o: works fine YEP

--write_to_movie: trash. delete. make behavior between renderers equal: default behavior should be rendering to a file. -i -> interactive preview. -p opens rendered file after finished process in video player.

--log_to_file is fine, that has use cases

--log_dir: is this really required? does anyone use this?

-n: useful, keep

-a: kind of niche, but probably keep?

--format: good one! (but why is it in the render options section, and not in the output section?)

-s: good one. keep.

-q presets: it would be nice if they would work/combine in a nicer way with resolution/fps changes, but they are widely used. keep.

-r: useful. Do not understand current explanation for it.

-g, -i should have been long deleted at this point

-s is documented twice for whatever reason

-t useful. green friend animations have been done with -t --format=webm

projection_fill/stroke_shaders: probably delete, not needed.

--jupyter: still needed?

@behackl behackl modified the milestones: v0.18.0, v0.18.1 Nov 3, 2023
@behackl behackl modified the milestones: v0.18.1, v0.19.0 Apr 24, 2024
@behackl
Copy link
Member

behackl commented Jul 13, 2024

I think this should be closed in favour of the approach that resolves #3466.

@behackl behackl closed this Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking changes This PR introduces breaking changes enhancement Additions and improvements in general help wanted We would appreciate help on this issue/PR maintenance refactoring, typos, removing clutter/dead code, and other code quality improvements
Projects
Status: Rejected
Development

Successfully merging this pull request may close these issues.

2 participants