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

Add PEP561 type marker #1583

Merged
merged 2 commits into from
Feb 26, 2020
Merged

Add PEP561 type marker #1583

merged 2 commits into from
Feb 26, 2020

Conversation

palango
Copy link
Contributor

@palango palango commented Feb 21, 2020

What was wrong?

Related to Issue #1582

How was it fixed?

This was my first try, but it doesn't seem to work. When running make dist I see the following line:

...
adding 'web3/net.py'
adding 'web3/parity.py'
adding 'web3/pm.py'
adding 'web3/py.typed'
adding 'web3/testing.py'
adding 'web3/types.py'
...

But when I later look into the source package, it's not there.

Todo:

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@kclowes
Copy link
Collaborator

kclowes commented Feb 21, 2020

@palango I'm pretty new to typing myself, but looking at a similar PR in the eth-abi package, I think what you have is all that you need to do. I ran a make sdist for eth-abi, and it doesn't show eth-abi/py.typed in the source package either. @carver do you have any other feedback/insight?

@carver

This comment has been minimized.

@kclowes

This comment has been minimized.

@cburgdorf
Copy link
Contributor

The change looks in fact correct. A simple way to verify it would be to install web3 at this commit via pip, import some web3 functions, call reveal_type(some_function) and run mypy on it. If it shows the correct types, it is working, if it reports Any it isn't.

@kclowes
Copy link
Collaborator

kclowes commented Feb 24, 2020

Aha! Thanks @cburgdorf!

@palango
Copy link
Contributor Author

palango commented Feb 25, 2020

Thanks @cburgdorf

I tested with a small script on a venv on my branch and master.

from web3 import Web3

a = Web3()
b = a.eth.getTransactionReceipt('test')

reveal_type(b)
reveal_type(a.eth.getTransactionReceipt)

Master

mypy test.py
test.py:1: error: Cannot find implementation or library stub for module named 'web3'
test.py:1: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
test.py:6: note: Revealed type is 'Any'
test.py:7: note: Revealed type is 'Any'
Found 1 error in 1 file (checked 1 source file)

This branch

 mypy test.py
test.py:4: error: Argument 1 to "getTransactionReceipt" of "Eth" has incompatible type "str"; expected "Union[Hash32, HexBytes, HexStr]"
test.py:6: note: Revealed type is 'TypedDict('web3.types.TxReceipt', {'blockHash': hexbytes.main.HexBytes, 'blockNumber': builtins.int, 'contractAddress': Union[eth_typing.evm.ChecksumAddress, None], 'cumulativeGasUsed': builtins.int, 'gasUsed': web3.types.Wei, 'from': eth_typing.evm.ChecksumAddress, 'logs': builtins.list[TypedDict('web3.types.LogReceipt', {'address': eth_typing.evm.ChecksumAddress, 'blockHash': hexbytes.main.HexBytes, 'blockNumber': eth_typing.evm.BlockNumber, 'data': eth_typing.encoding.HexStr, 'logIndex': builtins.int, 'payload': hexbytes.main.HexBytes, 'removed': builtins.bool, 'topic': hexbytes.main.HexBytes, 'topics': typing.Sequence[hexbytes.main.HexBytes], 'transactionHash': hexbytes.main.HexBytes, 'transactionIndex': builtins.int})], 'logsBloom': hexbytes.main.HexBytes, 'root': eth_typing.encoding.HexStr, 'status': builtins.int, 'to': eth_typing.evm.ChecksumAddress, 'transactionHash': hexbytes.main.HexBytes, 'transactionIndex': builtins.int})'
test.py:7: note: Revealed type is 'def (transaction_hash: Union[eth_typing.evm.Hash32, hexbytes.main.HexBytes, eth_typing.encoding.HexStr]) -> TypedDict('web3.types.TxReceipt', {'blockHash': hexbytes.main.HexBytes, 'blockNumber': builtins.int, 'contractAddress': Union[eth_typing.evm.ChecksumAddress, None], 'cumulativeGasUsed': builtins.int, 'gasUsed': web3.types.Wei, 'from': eth_typing.evm.ChecksumAddress, 'logs': builtins.list[TypedDict('web3.types.LogReceipt', {'address': eth_typing.evm.ChecksumAddress, 'blockHash': hexbytes.main.HexBytes, 'blockNumber': eth_typing.evm.BlockNumber, 'data': eth_typing.encoding.HexStr, 'logIndex': builtins.int, 'payload': hexbytes.main.HexBytes, 'removed': builtins.bool, 'topic': hexbytes.main.HexBytes, 'topics': typing.Sequence[hexbytes.main.HexBytes], 'transactionHash': hexbytes.main.HexBytes, 'transactionIndex': builtins.int})], 'logsBloom': hexbytes.main.HexBytes, 'root': eth_typing.encoding.HexStr, 'status': builtins.int, 'to': eth_typing.evm.ChecksumAddress, 'transactionHash': hexbytes.main.HexBytes, 'transactionIndex': builtins.int})'

So this seems to work as expected. Would it be a lot of effort to create a release with this change?

Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

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

Thanks @palango! And thanks for the input @carver and @cburgdorf!

@kclowes kclowes merged commit 1a4f53a into ethereum:master Feb 26, 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

Successfully merging this pull request may close these issues.

4 participants