-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
ENH Add micropip.uninstall() #55
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ryanking13 ! A couple of minor comments otherwise LGTM.
What would happen if we uninstall a package installed via loadPackage
? If it also uninstalls it, we need to be sure loadPackage
would be aware of it.
I think removing keys in |
@rth Thanks for the review! I think I addressed all of your comments. I changed the code a bit after your review (simplified how files are retrieved, and added tests), so I would appreciate it if you can review this one more time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor comment, otherwise LGTM. Thanks!
Related: #51
Add
micropip.uninstall()
method, which uninstalls packages that are installed.I used a quite simple approach for this compared to
pip uninstall
, since we only support installing wheels, and wheel metadata contain a list of files in the archive, so I just iterated through the list and removed them.I didn't add a rollback feature if uninstall fails in this PR. I'm not sure it is worth it, but probably we can add it as a follow up.