Skip to content

Conversation

@yairchu
Copy link
Contributor

@yairchu yairchu commented Oct 5, 2025

📝 Summary

This is a cleanup that deduplicates repeating code and in the process extracts the install_command method that will be needed by #6684

🔍 Description of Changes

Package manager's _install method contains duplication for all package managers except micropip. This change provides a default implementation that uses a new install_command method, both making the code more modular and less repetitive and preparing for future additional uses of the new method.

📋 Checklist

  • I have read the contributor guidelines.
  • I have added tests for the changes made.
    • As this is a refactoring, I didn't feel it was necessary, but I suppose I could add tests if ruled necessary.
  • I have run the code and verified that it works as expected.

@yairchu yairchu requested a review from dmadisetti as a code owner October 5, 2025 20:42
@vercel
Copy link

vercel bot commented Oct 5, 2025

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

Project Deployment Preview Comments Updated (UTC)
marimo-docs Ready Ready Preview Comment Oct 8, 2025 6:28pm

Copy link
Collaborator

@dmadisetti dmadisetti left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks, just one comment about a comment

@dmadisetti
Copy link
Collaborator

Also passing CI if you don't mind (just a few import deletions)

https://results.pre-commit.ci/run/github/678526156/1759731371.W4HPrCcLRxCk3SwUIr1C_w

@yairchu
Copy link
Contributor Author

yairchu commented Oct 8, 2025

Also passing CI if you don't mind (just a few import deletions)

https://results.pre-commit.ci/run/github/678526156/1759731371.W4HPrCcLRxCk3SwUIr1C_w

Done, as well as other typing errors etc

…thod

(a preparation for marimo-team#6684 and good organization regardless)
@yairchu
Copy link
Contributor Author

yairchu commented Oct 8, 2025

IIUC the failing CI test now is unrelated to this change

Copy link
Collaborator

@dmadisetti dmadisetti left a comment

Choose a reason for hiding this comment

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

Thanks!

@mscolnick mscolnick merged commit 941291e into marimo-team:main Oct 10, 2025
27 of 37 checks passed
@dmadisetti dmadisetti added the enhancement New feature or request label Oct 15, 2025
@yairchu yairchu deleted the install-commands branch October 15, 2025 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants