Skip to content

Conversation

@tlambert03
Copy link

List of Changes

  • add missing import re to utils/module_ops.py

Motivation

Selecting from multiple scene classes in a single file fails when providing a comma-separated list due to missing re import

❯ manim project/scene.py -pl
[10/07/20 15:46:14] WARNING  Option -l is deprecated please use the --quality/-q flag.
config_utils.py:660
1: SquareToCircle
2: SquareToCircle2
3: SquareToCircle3

Choose number corresponding to desired scene/arguments.
(Use comma separated list for multiple entries)
Choice(s):  1,2
Traceback (most recent call last):
  File "~/miniconda3/envs/manim/bin/manim", line 5, in <module>
    main()
  File "manim/manim/__main__.py", line 74, in main
    scene_classes_to_render = get_scenes_to_render(all_scene_classes)
  File "manim/manim/utils/module_ops.py", line 82, in get_scenes_to_render
    else prompt_user_for_choice(scene_classes)
  File "manim/manim/utils/module_ops.py", line 99, in prompt_user_for_choice
    for num_str in re.split(r"\s*,\s*", user_input.strip())
NameError: name 're' is not defined

Explanation for Changes

imports the missing library

Acknowledgement

@tlambert03 tlambert03 changed the title add re import add missing import Oct 7, 2020
@leotrs
Copy link
Contributor

leotrs commented Oct 7, 2020

@tlambert03 thanks for catching that!

@naveen521kk can you make sense of the error given by the black check? Sounds like the GH action is being flaky.

@huguesdevimeux is this something that we could easily add a test for?

@huguesdevimeux
Copy link
Member

Completly, this is typically something that need to be tested.
Could you read the wiki to see how to do ? You basically have to write a video test in test_scene_rendering/, following the instructions on the wiki.

@huguesdevimeux huguesdevimeux added the test requested Implementation of tests are requested label Oct 7, 2020
@huguesdevimeux huguesdevimeux marked this pull request as draft October 7, 2020 20:12
@leotrs
Copy link
Contributor

leotrs commented Oct 15, 2020

@tlambert03 sorry for dropping the ball here!

The current master has fixed this. I have added the missing test to #193

@leotrs leotrs closed this Oct 15, 2020
@leotrs leotrs mentioned this pull request Oct 15, 2020
15 tasks
@tlambert03 tlambert03 deleted the fix-prompt-user-choice branch October 15, 2020 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test requested Implementation of tests are requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants