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

Automatically identify the class name based on the specified line number. #2280

Merged
merged 6 commits into from
Dec 28, 2024

Conversation

Varniex
Copy link
Contributor

@Varniex Varniex commented Dec 16, 2024

Motivation

Whenever some user passes the command -e <line_number>, it feels redundant to specify the class name.

The closest class name (above the specified <line_number>) should automatically be identified by the program, and updates the value of scene_names key in the run_config dict.

Proposed Changes

I couldn't find a better position to insert the code than in the insert_embed_line_to_module function. As it ensures that the scene_names updates only when the e flag is passed, and also the module lines can be used to determine the class name.

Also, resolving minor bugs in text_mobject and string_mobject files.

manimlib/extract_scene.py Outdated Show resolved Hide resolved
manimlib/default_config.yml Outdated Show resolved Hide resolved
manimlib/extract_scene.py Outdated Show resolved Hide resolved
@@ -142,35 +143,48 @@ def get_indent(code_lines: list[str], line_number: int) -> str:
return n_spaces * " "


def insert_embed_line_to_module(module: Module, line_number: int):
def insert_embed_line_to_module(module: Module, run_config: Dict) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing run_config inside here, an alternative could be to return the scene names from this function and then update the scene_names of run_config from somewhere outside, e.g. in get_module that is calling insert_embed_line_to_module).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about the suggestion.
The reason I am searching for the scene name within this specific function is to access the module's lines to locate the keyword class above the specifed line number.

manimlib/default_config.yml Outdated Show resolved Hide resolved
manimlib/extract_scene.py Outdated Show resolved Hide resolved
@3b1b
Copy link
Owner

3b1b commented Dec 26, 2024

There are some cases where the scene that has the embed line is not the same as the one I'm running. For example, suppose you define some SceneA with a certain construct. Then you define SceneB as a subclass, which doesn't have its own construct method (so it effectively uses that of SceneA), but makes other changes to the class variables, methods, etc.
It may be that you want to drop an embed line in the construct method of SceneA, but to invoke SceneB.

The desired behavior is that if you do pass in a scene names, like manimgl file_name.py SceneB -se 123, then that scene name should take priority over whatever one lies above line number 123.

This should be a pretty quick fix, I believe all you need to do is check whether run_config already has scene_names defined, and if so, don't do the search for the scene.

@Varniex
Copy link
Contributor Author

Varniex commented Dec 28, 2024

Thanks @3b1b for the feedback.
I believe the recent changes is what you were suggesting.

@Varniex Varniex changed the title Automatically identifies the class name based on the specified line number. Automatically identify the class name based on the specified line number. Dec 28, 2024
@3b1b
Copy link
Owner

3b1b commented Dec 28, 2024

Looks good to me

@3b1b 3b1b merged commit 24eefef into 3b1b:master Dec 28, 2024
@Vegeorca
Copy link

Vegeorca commented Dec 28, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants