marimo icon indicating copy to clipboard operation
marimo copied to clipboard

Use ruff dependency graph for nested autoreloads

Open mthiboust opened this issue 4 weeks ago • 5 comments

📝 Summary

Fixes https://github.com/marimo-team/marimo/issues/6940

This PR is a proof-of-concept to make autoreload more robust by ensuring that when a module changes, all modules that depend on it (even indirectly) are also reloaded. This is achieved by leveraging Ruff’s static analysis, which provides a dependency graph. A positive side effect is that ruff analyze graph supports both absolute and relative imports.

As explained in the issue, ruff analyze graph is not working as expected for third-party packages in site-packages. Only current project and packages installed in editable mode are supported for automatic reload on file updates.

Got inspiration from this blog post: https://vincerose.dev/posts/ruff-test-filtering/

🔍 Description of Changes

The code could have been more straightforward but I chose to keep the existing code structure.

If you consider going further, we should decide whether to completely replace the existing, keep the two and use this new implementation as the default one, with the former one as a fallback?

I don't know how to unit test this feature.

📋 Checklist

  • [x] I have read the contributor guidelines.
  • [ ] For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
  • [ ] I have added tests for the changes made.
  • [x] I have run the code and verified that it works as expected.

mthiboust avatar Nov 04 '25 17:11 mthiboust

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
marimo-docs Ready Ready Preview Comment Nov 4, 2025 5:48pm

vercel[bot] avatar Nov 04 '25 17:11 vercel[bot]

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

github-actions[bot] avatar Nov 04 '25 17:11 github-actions[bot]

I have read the CLA Document and I hereby sign the CLA

mthiboust avatar Nov 04 '25 17:11 mthiboust

hey @mthiboust this may have a bit of churn on the PR/review. it might be easier to make the more incremental fixes/changes (like the fallback mentioned in this comment)

mscolnick avatar Nov 04 '25 20:11 mscolnick

hey @mthiboust this may have a bit of churn on the PR/review. it might be easier to make the more incremental fixes/changes (like the fallback mentioned in this comment)

This PR was more a proof-of-concept for now. I clarified the current status of the issue in this recent comment. In short, I don't think that the discussed fallback for relative imports will be sufficient to be useful if we are still lacking support for nested imports.

And implementing support for nested imports via ruff will avoid the relative import issue anyway because ruff analyze graph already supports both absolute and relative imports.

mthiboust avatar Nov 05 '25 10:11 mthiboust