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

Have autoreload update shell namespace with reloaded module variables #2278

Merged
merged 2 commits into from
Dec 13, 2024

Conversation

3b1b
Copy link
Owner

@3b1b 3b1b commented Dec 13, 2024

In response to #2275

When --autoreload is set, during the interactive mode, before each cell execution the user's module will be reloaded.

@Splines, is this what you had in mind? At the moment I removed the calls to shell.magic("autoreload all") because it feels like this accomplishes the same, but it's very possible I'm missing something.

@Splines
Copy link
Contributor

Splines commented Dec 13, 2024

Nice, this is indeed exactly what I had in mind, it works perfectly, thank you 🙏 And yes, I think we don't need the IPython magic anymore since this is doing what we need. Maybe there are some exotic cases where autoreload does something we don't do, but in my basic playing around with this feature I haven't found it yet.

One thing to note is that while adding import statements on the fly works, removing them doesn't have any effect and they are still cached. A simple reload() works in this case. Maybe we could delete the old module namespace before updating it to mitigate this? However, if there's no easy solution to this very edge case, I really don't mind and I can just as well use reload() once in a while 😅

Furthermore, note that the pipeline in #2268 failed such that the --autoreload CLI flag is not yet published to the docs here.

@@ -143,8 +143,11 @@ def reload_scene(self, embed_line: int | None = None) -> None:

def auto_reload(self):
"""Enables IPython autoreload for automatic reloading of modules."""
Copy link
Contributor

Choose a reason for hiding this comment

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

might want to adjust the docstring to make clear we don't use the %autoreload IPython line magic and instead reload user modules with our custom logic

@3b1b
Copy link
Owner Author

3b1b commented Dec 13, 2024

Maybe we could delete the old module namespace before updating it to mitigate this? However, if there's no easy solution to this very edge case, I really don't mind and I can just as well use reload() once in a while

That would be tricky, and prone to producing hard-to-detect errors. If you set up a keyboard shortcut for calling reload with the line number where the cursor is, it's really much of a cost at all.

I am less familiar with the docs build system. If you had any interest in looking into a fix, I'd certainly be grateful.

@3b1b 3b1b merged commit 39fbb67 into master Dec 13, 2024
0 of 2 checks passed
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