-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
Better modularization + dependency injection #112
Comments
cc @jaraco |
This approach sounds reasonable. I like the idea of essentially creating hooks that a programmer can override. My guess is that these hooks are going to demand some pretty intimate access to the state in Micropip. To the extent they can be stateless functions, that'd be great, but doing so would drastically limit the ability of that signature to evolve. The first step is obviously going to be to dogfood the concept with the standard implementation. It'll be interesting to see what interfaces emerge from that work and how clean those interfaces can be. Another possible way for programmers to extend/override functionality could be to offer overridable methods, e.g.: class Micropip:
def resolve(self, package):
...
def install(self, package):
...
class Backtracking:
"""Mix-in for Micropip to implement a backtracking resolver."""
def resolve(self, package):
# doesn't call super()
...
class BacktrackingMicropip(Backtracking, Micropip):
pass From a programmer's ergonomics, passing in functions or objects may seem nicer, but from a maintenance perspective of the reference implementation, it can be nice not to have the extra layer of abstraction. One nitpick - if possible as part of this work, it'd be nice to have another name for |
Sounds reasonable to me. I am not a big fan of Mixins, but subclassing can be a good idea. If this function needs to reference other state or methods in the micropip module, this would be a better way to do it.
Sure, I think we can use some other names like |
@RulerOfCakes This can be an interesting work if you are interested. |
Sure, I'm interested 👀 I'll see what I can do.. |
Thanks! I think we can start with defining |
micropip is used in many different downstream projects and has many different requirements, so I think it would be nice to 1) modularize overall micropip behavior, and 2) use dependency injection to customize micropip behavior.
micropip.Micropip()
Currently, micropip only has a global state and can only be used with global APIs like
micropip.install
andmicropip.freeze
.This issue proposes to define a
Micropip
class in addition to this, and make it possible to create instances with local state.The existing APIs will still be available (no breaking changes), and they will internally reference the default Micropip instance.
Dependency injection
When instantiating the Micropip instance, we can accept several parameters including,
resolver
,installer
, etc.This will make it easier to tweak micropip's behavior, which will make it easier for us to implement new features, or people can implement their own features. For instance, someone can make a backtracking resolver and replace our simple resolver if they want, or it might be even possible to use micropip in non-Pyodide environments.
Required works
The global state of micropips, which is currently jumbled in many ways, needs to be better categorized by purpose and defined in a clear API.
The text was updated successfully, but these errors were encountered: