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

Missed dependency when import web3: typing_extensions #1540

Closed
eugene-vasilev opened this issue Dec 6, 2019 · 11 comments
Closed

Missed dependency when import web3: typing_extensions #1540

eugene-vasilev opened this issue Dec 6, 2019 · 11 comments

Comments

@eugene-vasilev
Copy link

  • Version: latest
  • Python: 3.7
  • OS: linux
  • pip freeze output
attrdict==2.0.1
attrs==19.3.0
base58==1.0.3
certifi==2019.11.28
chardet==3.0.4
cytoolz==0.10.1
eth-abi==2.0.0
eth-account==0.4.0
eth-hash==0.2.0
eth-keyfile==0.5.1
eth-keys==0.2.4
eth-rlp==0.1.2
eth-typing==2.2.1
eth-utils==1.8.4
hexbytes==0.2.0
idna==2.8
importlib-metadata==1.2.0
ipfshttpclient==0.4.12
jsonschema==3.2.0
lru-dict==1.1.6
more-itertools==8.0.1
multiaddr==0.0.8
mypy-extensions==0.4.3
netaddr==0.7.19
parsimonious==0.8.1
pkg-resources==0.0.0
protobuf==3.11.1
pycryptodome==3.9.4
pyrsistent==0.15.6
requests==2.22.0
rlp==1.2.0
six==1.13.0
toolz==0.10.0
urllib3==1.25.7
varint==1.0.2
web3==5.3.1
websockets==8.1
zipp==0.6.0

What was wrong?

When I want to import web3, I catch the following error:

File "/home/evasilev/Documents/Projects/pytests/common/base_test.py", line 14, in
from web3 import Web3
File "/home/evasilev/Documents/Projects/pytests/venv/lib/python3.7/site-packages/web3/init.py", line 9, in
from web3.main import (
File "/home/evasilev/Documents/Projects/pytests/venv/lib/python3.7/site-packages/web3/main.py", line 27, in
from web3._utils.abi import (
File "/home/evasilev/Documents/Projects/pytests/venv/lib/python3.7/site-packages/web3/_utils/abi.py", line 70, in
from web3._utils.ens import (
File "/home/evasilev/Documents/Projects/pytests/venv/lib/python3.7/site-packages/web3/_utils/ens.py", line 22, in
from web3.exceptions import (
File "/home/evasilev/Documents/Projects/pytests/venv/lib/python3.7/site-packages/web3/exceptions.py", line 4, in
from web3.types import (
File "/home/evasilev/Documents/Projects/pytests/venv/lib/python3.7/site-packages/web3/types.py", line 23, in
from typing_extensions import (
ModuleNotFoundError: No module named 'typing_extensions'

I found that this dependency is used in the files:

  • web3/_utils/compat/init.py
  • web3/types.py

How can it be fixed?

Add typing_extensions library into setup.py file to install_requires list (line 70-87)

@kclowes
Copy link
Collaborator

kclowes commented Dec 6, 2019

Thank you for pointing this out! I will open a PR shortly!

@pipermerriam
Copy link
Member

@kclowes we have some tests in trinity that actually build the wheel, install it, and then run some tests against it. Maybe that type of thing would be useful here as it would catch this type of thing.

@kclowes
Copy link
Collaborator

kclowes commented Dec 6, 2019

That would be super useful. I'll check it out!

@cheeseandcereal
Copy link

cheeseandcereal commented Dec 6, 2019

Just as a thought, typing extensions should really never be required at runtime, so rather than simply adding typing_extensions as a requirement (which is ok for just a short-term solution), I would advocate for surrounding any usage of types with if TYPE_CHECKING:

Python type checking is only for static type checking, so it doesn't need to (and shouldn't) be required at runtime.

Taking a quick look, this is already done for some types, but not all of them i.e:

web3.py/web3/main.py

Lines 98 to 107 in 0aa0766

from web3.types import ( # noqa: F401
Middleware,
MiddlewareOnion,
)
from web3.version import (
Version,
)
if TYPE_CHECKING:
from web3.pm import PM # noqa: F401

Making sure that all typing imports are behind an if TYPE_CHECKING: block would remove the need for the typing_extensions package at runtime, which would help reduce unnecessary bloat.

@kclowes
Copy link
Collaborator

kclowes commented Dec 6, 2019

You're right, @cheeseandcereal, that seems like a better fix. This issue has the quick fix implemented in 5.4.0 by adding typing_extensions to the required packages, but I'm going to leave it open to add the TYPE_CHECKING check where it's missing. @njgheorghita do you have opinions?

@njgheorghita
Copy link
Contributor

Yup, this is most likely my fault from all the type-stuff I've been doing. So there's web3._utils.compat which is an attempt to get around this. Basically there are three types Literal, Protocol, and TypedDict which are available in the standard typing module in python>=3.8. But, since web3 is python>=3.6 right now, I added the types in web3._utils.compat to try and mitigate any conflicts.

However, say a user is running python 3.6/3.7, then they need to have typing_extensions installed if they want to do any type checking. Though, granted as @cheeseandcereal pointed out, this is not required for runtime. . .

However, hiding all the extended types behind an if TYPE_CHECKING is possible, but adds complexity - and has some tricky edge cases when using the Protocol type. So, all this to say that my best suggestion is here. We can ditch the mypy_extensions dependency now, and remove the typing_extensions dependency when web3.py requires python>=3.8. Imo the bloat from typing_extensions isn't ideal, but worth it until we reach 3.8 to avoid the tricky edge cases - but i'm open to suggestions.

@cburgdorf
Copy link
Contributor

Just as a thought, typing extensions should really never be required at runtime

That's not exactly true though.

The typing_extension package aggregates features that might be added to the std lib typing namespace in a future upcoming version. E.g. it does host things like Protocol which you can use to implement protocols such as:

class SupportsError(Protocol):
    error: Exception

If I'm not mistaking I think this is added in Python 3.8 so that Protocol can directly be imported from typing in the same way like e.g. NamedTuple which you'd use like this:

class SyncProgress(NamedTuple):
    starting_block: BlockNumber
    current_block: BlockNumber
    highest_block: BlockNumber

And you will absolutely need to have this at runtime because you would use it in regular function bodies such as:

    def update_current_block(self, new_current_block: BlockNumber) -> SyncProgress:
        return SyncProgress(self.starting_block, new_current_block, self.highest_block)

@cheeseandcereal
Copy link

cheeseandcereal commented Dec 9, 2019

@cburgdorf I agree that namedtuple would be an exception, but that's only because a namedtuple is a literal collections object type.

A python protocol should never need to be imported or defined at runtime in order to take advantage of its typing features.
Take the following example, where we have a folder with __init__.py, mytypes.py, and main.py:

(__init__.py is empty)

mytypes.py

from typing_extensions import Protocol
# from typing import Protocol     <- Python 3.8+

class MyProto(Protocol):
    def some_method(self) -> int:
        ...

main.py

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from mytypes import MyProto

class MyImplementation():
    def some_method(self) -> int:
        return 0

def do_the_thing(x: "MyProto") -> int:
    return x.some_method()

if __name__ == "__main__":
    my_obj = MyImplementation()
    print(do_the_thing(my_obj) + 1)

This gives us the static type checking advantage of protocols, without ever having to use them at runtime.

The trick is that all of your types, including protocols need to be defined in a separate file which is only ever imported behind an if TYPE_CHECKING block

@cburgdorf
Copy link
Contributor

A python protocol should never need to be imported or defined at runtime in order to take advantage of its typing features

I think it depends. Protocols even allow to implement default implementations

Also, you may want to check isinstance against protocols at runtime.

I personally think it's a safer default to not treat mypy_extensions as something that can be omitted at runtime.

@cheeseandcereal
Copy link

cheeseandcereal commented Dec 9, 2019

I suppose that is true. At least in that case, typing_extensions should replace mypy_extensions, and only be required on python < 3.8

@kclowes
Copy link
Collaborator

kclowes commented Oct 30, 2020

Closing, this was fixed a long time ago in #1546.

@kclowes kclowes closed this as completed Oct 30, 2020
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

No branches or pull requests

6 participants