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

MAINT micropip: move most methods from PackageManager to Transaction and remove PackageManager #2565

Merged
merged 4 commits into from May 17, 2022

Conversation

hoodmane
Copy link
Member

This is more reorganization of micropip to try to make the logic easier to follow. I turned INSTALLED_PACKAGES into a global variable. I turned _install into a top level function definition and merged it with install rather than a class method. The other methods of PackageManager turned into Transaction methods. I removed PackageManager entirely.

@hoodmane
Copy link
Member Author

I'm not really sure why matplotlib fails to build on this branch...

Copy link
Member

@ryanking13 ryanking13 left a comment

Choose a reason for hiding this comment

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

I turned _install into a top level function definition and merged it with install rather than a class method.

I was always thinking of this but never worked on it, thanks @hoodmane!
I didn't review the detail, I guess code details are not changed much right?

There is a failure in CI, but the matplotlib build failure doesn't seem related to this PR.

@hoodmane
Copy link
Member Author

Yeah it seems that matplotlib is failing on a bunch of my branches with micropip-related changes.

code details are not changed much right?

No, it's just a refactoring. I consolidated PackageManager.install with install functions, moved the other methods from PackageManager to Transaction, and moved PackageManager.installed_packages to INSTALLED_PACKAGES global variables.

@hoodmane
Copy link
Member Author

Thanks for the review @ryanking13!

@hoodmane hoodmane merged commit c4cc406 into pyodide:main May 17, 2022
@hoodmane hoodmane deleted the micropip-transaction-methods branch May 17, 2022 01:10
@ryanking13
Copy link
Member

ryanking13 commented May 17, 2022

It seems like recent release of setuptools is causing an error #2567

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.

None yet

2 participants