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

Add autoreload mechanism #2252

Closed
wants to merge 7 commits into from
Closed

Conversation

Splines
Copy link
Contributor

@Splines Splines commented Nov 28, 2024

🎈 Motivation

IPython offers an %autoreload all magic command:

autoreload reloads modules automatically before entering the execution of code typed at the IPython prompt.

This makes it possible for Manim users to make changes in imported files and see those changes reflected automatically without having to reload manually (by means of quitting and restarting the terminal, or by means of the new reload() command from #2251). Therefore, this PR improves the interactive experience with Manim fostering playful change of parameters/functions etc.

💻 Proposed changes

  • Add a new --autoreload CLI flag.
  • If that flag is set, call the respective magic commands before the InteractiveShellEmbed shell is started.

Note that autoreload should probably not be activated by default because it will slow down the performance since modules will be reloaded automatically and maybe even in cases where it wouldn't be necessary. Furthermore, we should be aware of the caveats that come with autoreload. This is why I went for exposing it as flag that can be activated if users like to.

🧪 Test

# file a.py
import sys

sys.path.append(".")
from manimlib import *

from b import B

class AScene(Scene):
    def construct(self):
        ## Starting
        print("Hello from AScene")

        ## B
        b = B()
        print(b.answer_to_life())
# file b.py
class B:
    def answer_to_life(self):
        return 10
# Line 13 is where the comment "## B" is
manimgl "/path/to/a.py" AScene -se 13 --autoreload
  • Preview by starting from the second comment (## B) and get 10 printed to the console.
  • Change 10 to 42 in b.py and save the file
  • Preview again and see 42 in the console, without having to reload manually 🎉

Tip

Unfortunately, classes inside the same file won't be autoreloaded, just modules that are imported. E.g. see the evolute test example provided in #2240. So when defining custom classes that don't just host Manim rendering logic like self.play() etc., it might be worth to outsource such parts to another file when we want to work on them and make lots of changes. Otherwise, starting with #2251, the reload() command such also work in such cases.

If you want to reload classes from other modules, I just found this post.

📜 Documentation

I've added the CLI flag --autoreload to the docs.

I also tried to build the docs but didn't succeed

I wanted to test this by following your instructions from here. Unfortunately, I stumbled into these issues:

Jinja

I first got this error and therefore downgraded Jinja:

$ pip install "jinja2<3.1"

> Successfully uninstalled Jinja2-3.1.4
> Successfully installed jinja2-3.0.3

Sphinx

Then I tried to run make html again which failed with:

WARNING: while setting up extension sphinx.addnodes: node class 'meta' is already registered, its visitors will be overridden

Sphinx version error:
The sphinxcontrib.applehelp extension used by this project needs at least Sphinx v5.0; it therefore cannot be built with this version.

Realizing that Sphinx==3.0.3 is specified in docs/requirements.txt, I quit this pursuit to build the docs as I was not even sure it would work on your end...

@Splines
Copy link
Contributor Author

Splines commented Dec 10, 2024

Nice, the new scene_embed makes it a lot easier to get a grasp on what is going on! I've now made my PR ready for the new changes. (Also note my review comment here where I describe how tempfile doesn't work for me. Note that GitHub is slow and you have to wait a few seconds after clicking the link...)

@3b1b
Copy link
Owner

3b1b commented Dec 10, 2024

This looks useful. I'm in the midst of refactoring how the global configuration works, which may affect this. I'll probably incorporate the changes here into a new PR once I'm finished.

@3b1b 3b1b mentioned this pull request Dec 12, 2024
@Splines Splines closed this Dec 12, 2024
@Splines Splines deleted the feature/autoreload branch December 12, 2024 22:24
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.

2 participants