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
Add support for external modules to web3 instance #2288
Conversation
b26de80
to
d550b85
Compare
@@ -18,11 +18,13 @@ | |||
|
|||
def attach_modules( | |||
parent_module: Union["Web3", "Module"], | |||
module_definitions: Dict[str, Sequence[Any]], | |||
module_definitions: Dict[str, Any], |
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.
The recursive nature of this method makes it really hard to add any useful typing here so this has to be Any
unfortunately. But I am new to typing in python, maybe there's a way to make this stricter 🤔 . At least all the uses of this method are able to be a tad more strict.
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.
I found this documentation: https://mypy.readthedocs.io/en/stable/protocols.html#recursive-protocols, but I would have to play with it some to figure out how exactly we'd use it. If digging into that seems out of the scope of this PR, I'm fine leaving this as-is.
d550b85
to
ee6c2e5
Compare
- added web3.attach_module() to attach a single module to a web3 instance e.g. w3.attach_module('module1', ModuleClass1) - added support for initializing the instance with external modules e.g. w3 = Web3(..., external_modules={'module1': ModuleClass1}) - modules can be passed in without needing to be a tuple or list if they don't contain submodules i.e. pass in the class by itself instead a tuple with 1 argument, (ModuleClass1,) - added tests for the above - updated docs for web3 main to include usage examples for the above changes
ee6c2e5
to
1190d9d
Compare
@@ -18,11 +18,13 @@ | |||
|
|||
def attach_modules( | |||
parent_module: Union["Web3", "Module"], | |||
module_definitions: Dict[str, Sequence[Any]], | |||
module_definitions: Dict[str, Any], |
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.
I found this documentation: https://mypy.readthedocs.io/en/stable/protocols.html#recursive-protocols, but I would have to play with it some to figure out how exactly we'd use it. If digging into that seems out of the scope of this PR, I'm fine leaving this as-is.
- Add a test to make sure a tuple with a single module class still works for backwards compatibility. - Take out the docstring examples for `attach_module()` and leave them only in the docs. The consensus is that lengthy docstrings reiterating what is in the documentation would just crowd out our files and make the code harder to read. Instead, we should opt for brief yet useful despritions in the method docstrings.
- Add a test to make sure a tuple with a single module class still works for backwards compatibility. - Take out the docstring examples for `attach_modules()` and leave them only in the docs. The consensus is that lengthy docstrings reiterating what is in the documentation would just crowd out our files and make the code harder to read. Instead, we should opt for brief yet useful despritions in the method docstrings. - Change the method to allow for a dict as the argument, as is in the constructor, so that multiple modules may be attached if desired.
8920d55
to
971028d
Compare
- Add a test to make sure a tuple with a single module class still works for backwards compatibility. - Take out the docstring examples for `attach_modules()` and leave them only in the docs. The consensus is that lengthy docstrings reiterating what is in the documentation would just crowd out our files and make the code harder to read. Instead, we should opt for brief yet useful despritions in the method docstrings. - Change the method to allow for a dict as the argument, as is in the constructor, so that multiple modules may be attached if desired.
971028d
to
71d9de9
Compare
- Add a test to make sure a tuple with a single module class still works for backwards compatibility. - Take out the docstring examples for `attach_modules()` and leave them only in the docs. The consensus is that lengthy docstrings reiterating what is in the documentation would just crowd out our files and make the code harder to read. Instead, we should opt for brief yet useful despritions in the method docstrings. - Change the method to allow for a dict as the argument, as is in the constructor, so that multiple modules may be attached if desired.
71d9de9
to
183a5dc
Compare
- Add a test to make sure a tuple with a single module class still works for backwards compatibility. - Take out the docstring examples for `attach_modules()` and leave them only in the docs. The consensus is that lengthy docstrings reiterating what is in the documentation would just crowd out our files and make the code harder to read. Instead, we should opt for brief yet useful despritions in the method docstrings. - Change the method to allow for a dict as the argument, as is in the constructor, so that multiple modules may be attached if desired. - Use the attach_modules utility method as an underscore-aliased internal method for clarity
183a5dc
to
95bd986
Compare
What was wrong?
We don't have an easy way to attach modules to the
Web3
instance. This will help users customize their web3 experience but can also help to support things like attaching layer 2 modules from a separate library or any other modules that can prove useful.Closes Difficult to attach one extra module to a web3 instance.. #2231
How was it fixed?
The idea is to just expose the internal api for now. Perhaps improve on this in v6.
Web3
instantiation. This is separate from the internal default modules already attached at instantiation.attach_modules()
methodTodo:
Cute Animal Picture