Properly initialize external modules

docs/web3.main.rst

@@ -419,17 +419,13 @@ web3.py library.
 External Modules
 ~~~~~~~~~~~~~~~~
 
-External modules can be used to introduce custom or third-party APIs to your ``Web3`` instance. Adding external modules
-can occur either at instantiation of the ``Web3`` instance or by making use of the ``attach_modules()`` method.
-
-Unlike the native modules, external modules need not inherit from the ``web3.module.Module`` class. The only requirement
-is that a Module must be a class and, if you'd like to make use of the parent ``Web3`` instance, it must be passed into
-the ``__init__`` function. For example:
+External modules can be used to introduce custom or third-party APIs to your ``Web3`` instance. External modules are simply
+classes whose methods and properties can be made available within the ``Web3`` instance. Optionally, the external module may
+make use of the parent ``Web3`` instance by accepting it as the first argument within the ``__init__`` function:
 
 .. code-block:: python
 
-    >>> class ExampleModule():
-    ...
+    >>> class ExampleModule:
     ...     def __init__(self, w3):
     ...         self.w3 = w3
     ...
@@ -440,7 +436,9 @@ the ``__init__`` function. For example:
 .. warning:: Given the flexibility of external modules, use caution and only import modules from trusted third parties
    and open source code you've vetted!
 
-To instantiate the ``Web3`` instance with external modules:
+Configuring external modules can occur either at instantiation of the ``Web3`` instance or by making use of the
+``attach_modules()`` method. To instantiate the ``Web3`` instance with external modules use the ``external_modules``
+keyword argument:
 
 .. code-block:: python
 
@@ -466,11 +464,11 @@ To instantiate the ``Web3`` instance with external modules:
     ... )
 
     # `return_zero`, in this case, is an example attribute of the `ModuleClass1` object
-    >>> w3.module1.return_zero
+    >>> w3.module1.return_zero()
     0
-    >>> w3.module2.submodule1.return_one
+    >>> w3.module2.submodule1.return_one()
     1
-    >>> w3.module2.submodule2.submodule2a.return_two
+    >>> w3.module2.submodule2.submodule2a.return_two()
     2
 
 
@@ -504,9 +502,9 @@ To instantiate the ``Web3`` instance with external modules:
         ...         })
         ...     })
         ... })
-        >>> w3.module1.return_zero
+        >>> w3.module1.return_zero()
         0
-        >>> w3.module2.submodule1.return_one
+        >>> w3.module2.submodule1.return_one()
         1
-        >>> w3.module2.submodule2.submodule2a.return_two
+        >>> w3.module2.submodule2.submodule2a.return_two()
         2

newsfragments/2328.bugfix.rst

@@ -0,0 +1 @@
+Properly initialize external modules that do not inherit from the ``web3.module.Module`` class

tests/core/conftest.py

@@ -48,31 +48,52 @@ class Module4(Module):
 
 @pytest.fixture(scope='module')
 def module1_unique():
+    # uses ``Web3`` instance by accepting it as first arg in the ``__init__()`` method
     class Module1:
         a = 'a'
+
+        def __init__(self, w3):
+            self._b = "b"
+            self.w3 = w3
+
+        def b(self):
+            return self._b
+
+        @property
+        def return_eth_chain_id(self):
+            return self.w3.eth.chain_id
     return Module1
 
 
 @pytest.fixture(scope='module')
 def module2_unique():
     class Module2:
-        b = 'b'
+        c = 'c'
 
         @staticmethod
-        def c():
-            return 'c'
+        def d():
+            return 'd'
     return Module2
 
 
 @pytest.fixture(scope='module')
 def module3_unique():
     class Module3:
-        d = 'd'
+        e = 'e'
     return Module3
 
 
 @pytest.fixture(scope='module')
 def module4_unique():
     class Module4:
-        e = 'e'
+        f = 'f'
     return Module4
+
+
+@pytest.fixture(scope='module')
+def module_many_init_args():
+    class ModuleManyArgs:
+        def __init__(self, a, b):
+            self.a = a
+            self.b = b
+    return ModuleManyArgs

tests/core/utilities/test_attach_modules.py

@@ -1,3 +1,6 @@
+from io import (
+    UnsupportedOperation,
+)
 import pytest
 
 from eth_utils import (
@@ -147,18 +150,34 @@ def test_attach_external_modules_that_do_not_inherit_from_module_class(
     # assert module1 attached
     assert hasattr(w3, 'module1')
     assert w3.module1.a == 'a'
+    assert w3.module1.b() == 'b'
+    assert w3.module1.return_eth_chain_id == w3.eth.chain_id
 
     # assert module2 + submodules attached
     assert hasattr(w3, 'module2')
-    assert w3.module2.b == 'b'
-    assert w3.module2.c() == 'c'
+    assert w3.module2.c == 'c'
+    assert w3.module2.d() == 'd'
 
     assert hasattr(w3.module2, 'submodule1')
-    assert w3.module2.submodule1.d == 'd'
+    assert w3.module2.submodule1.e == 'e'
     assert hasattr(w3.module2.submodule1, 'submodule2')
-    assert w3.module2.submodule1.submodule2.e == 'e'
+    assert w3.module2.submodule1.submodule2.f == 'f'
 
     # assert default modules intact
     assert hasattr(w3, 'geth')
     assert hasattr(w3, 'eth')
     assert is_integer(w3.eth.chain_id)
+
+
+def test_attach_modules_for_module_with_more_than_one_init_argument(web3, module_many_init_args):
+    with pytest.raises(
+        UnsupportedOperation,
+        match=(
+            "A module class may accept a single `Web3` instance as the first argument of its "
+            "__init__\\(\\) method. More than one argument found for ModuleManyArgs: \\['a', 'b']"
+        )
+    ):
+        Web3(
+            EthereumTesterProvider(),
+            external_modules={'module_should_fail': module_many_init_args}
+        )

tests/core/web3-module/test_attach_modules.py

@@ -1,3 +1,8 @@
+from io import (
+    UnsupportedOperation,
+)
+import pytest
+
 from eth_utils import (
     is_integer,
 )
@@ -51,18 +56,31 @@ def test_attach_modules_that_do_not_inherit_from_module_class(
     # assert module1 attached
     assert hasattr(web3, 'module1')
     assert web3.module1.a == 'a'
+    assert web3.module1.b() == 'b'
+    assert web3.module1.return_eth_chain_id == web3.eth.chain_id
 
     # assert module2 + submodules attached
     assert hasattr(web3, 'module2')
-    assert web3.module2.b == 'b'
-    assert web3.module2.c() == 'c'
+    assert web3.module2.c == 'c'
+    assert web3.module2.d() == 'd'
 
     assert hasattr(web3.module2, 'submodule1')
-    assert web3.module2.submodule1.d == 'd'
+    assert web3.module2.submodule1.e == 'e'
     assert hasattr(web3.module2.submodule1, 'submodule2')
-    assert web3.module2.submodule1.submodule2.e == 'e'
+    assert web3.module2.submodule1.submodule2.f == 'f'
 
     # assert default modules intact
     assert hasattr(web3, 'geth')
     assert hasattr(web3, 'eth')
     assert is_integer(web3.eth.chain_id)
+
+
+def test_attach_modules_for_module_with_more_than_one_init_argument(web3, module_many_init_args):
+    with pytest.raises(
+        UnsupportedOperation,
+        match=(
+            "A module class may accept a single `Web3` instance as the first argument of its "
+            "__init__\\(\\) method. More than one argument found for ModuleManyArgs: \\['a', 'b']"
+        )
+    ):
+        web3.attach_modules({'module_should_fail': module_many_init_args})

web3/_utils/module.py

@@ -1,7 +1,12 @@
+import inspect
+from io import (
+    UnsupportedOperation,
+)
 from typing import (
     TYPE_CHECKING,
     Any,
     Dict,
+    List,
     Optional,
     Sequence,
     Union,
@@ -18,6 +23,22 @@
     from web3 import Web3  # noqa: F401
 
 
+def _validate_init_params_and_return_if_found(module_class: Any) -> List[str]:
+    init_params_raw = list(inspect.signature(module_class.__init__).parameters)
+    module_init_params = [
+        param for param in init_params_raw if param not in ['self', 'args', 'kwargs']
+    ]
+
+    if len(module_init_params) > 1:
+        raise UnsupportedOperation(
+            "A module class may accept a single `Web3` instance as the first argument of its "
+            f"__init__() method. More than one argument found for {module_class.__name__}: "
+            f"{module_init_params}"
+        )
+
+    return module_init_params
+
+
 def attach_modules(
     parent_module: Union["Web3", "Module"],
     module_definitions: Dict[str, Any],
@@ -34,18 +55,20 @@ def attach_modules(
                 "already has an attribute with that name"
             )
 
-        if issubclass(module_class, Module):
-            # If the `module_class` inherits from the `web3.module.Module` class, it has access to
-            # caller functions internal to the web3.py library and sets up a proper codec. This
-            # is likely important for all modules internal to the library.
-            if w3 is None:
-                setattr(parent_module, module_name, module_class(parent_module))
-                w3 = parent_module
-            else:
-                setattr(parent_module, module_name, module_class(w3))
+        # The parent module is the ``Web3`` instance on first run of the loop
+        if type(parent_module).__name__ == 'Web3':
+            w3 = parent_module
+
+        module_init_params = _validate_init_params_and_return_if_found(module_class)
+        if len(module_init_params) == 1:
+            # Modules that need access to the ``Web3`` instance may accept the instance as the first
+            # arg in their ``__init__()`` method. This is the case for any module that inherits from
+            # ``web3.module.Module``.
+            # e.g. def __init__(self, w3):
+            setattr(parent_module, module_name, module_class(w3))
         else:
-            # An external `module_class` need not inherit from the `web3.module.Module` class.
-            setattr(parent_module, module_name, module_class)
+            # Modules need not take in a ``Web3`` instance in their ``__init__()`` if not needed
+            setattr(parent_module, module_name, module_class())
 
         if module_info_is_list_like:
             if len(module_info) == 2: