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

Support type hints in ethpm module #1492

Merged
merged 3 commits into from Nov 6, 2019

Conversation

@njgheorghita
Copy link
Contributor

njgheorghita commented Nov 4, 2019

What was wrong?

Type hints in ethpm module.

Todo:

Cute Animal Picture

image

@njgheorghita njgheorghita force-pushed the njgheorghita:type-hints-ethpm branch from d6577da to 2662668 Nov 4, 2019
@njgheorghita njgheorghita requested review from pipermerriam and kclowes Nov 4, 2019
Copy link
Member

pipermerriam left a comment

One thing that needs to be addressed (the kwargs thing)

@@ -51,7 +51,7 @@ def __init__(self, address: bytes, **kwargs: Any) -> None:
"Contract cannot be instantiated until its bytecode is linked."
)
validate_address(address)
super(LinkableContract, self).__init__(address=address, **kwargs)

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Nov 4, 2019

Member

Doesn't seem like it would be ok to drop the **kwargs here. That would silently ignore any additional parameters beyond address (like abi, bytecode)?

@@ -371,7 +374,9 @@ def deployments(self) -> Union["Deployments", Dict[None, None]]:
return Deployments(deployments, all_contract_instances, self.w3)

@to_dict
def _get_all_contract_instances(self, deployments):
def _get_all_contract_instances(
self, deployments: Dict[str, Any]

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Nov 4, 2019

Member

Aren't deployments structured? Can you define a TypedDict in place of the Any?

try:
from typing import TypedDict # type: ignore
except ImportError:
from mypy_extensions import TypedDict # <=3.7

Comment on lines 21 to 25

This comment has been minimized.

Copy link
@njgheorghita

njgheorghita Nov 5, 2019

Author Contributor

@pipermerriam I'm not sure if this is the best way to handle this, since web3 will be supporting 3.8 soon, where TypedDict is automatically included with typing? Is it better to just go with the mypy_extensions import for now and add mypy_extensions to setup.py['install_requires']?

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Nov 5, 2019

Member

This is generally an acceptable pattern. If you find yourself copying these import lines across multiple files I'd recommend putting it somewhere like library._compat and then importing from there everywhere else.

@njgheorghita njgheorghita force-pushed the njgheorghita:type-hints-ethpm branch from 5f8ccd6 to f0b7734 Nov 6, 2019
@njgheorghita njgheorghita merged commit 839b0d0 into ethereum:master Nov 6, 2019
42 checks passed
42 checks passed
ci/circleci: docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: py36-core Your tests passed on CircleCI!
Details
ci/circleci: py36-ens Your tests passed on CircleCI!
Details
ci/circleci: py36-ethpm Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-ethtester-pyevm Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-http-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-http-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-ipc-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-ipc-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-ws-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-ws-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-parity-http Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-parity-ipc Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-parity-ws Your tests passed on CircleCI!
Details
ci/circleci: py37-core Your tests passed on CircleCI!
Details
ci/circleci: py37-ens Your tests passed on CircleCI!
Details
ci/circleci: py37-ethpm Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-ethtester-pyevm Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-http-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-http-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-ipc-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-ipc-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-ws-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-ws-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-parity-http Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-parity-ipc Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-parity-ws Your tests passed on CircleCI!
Details
ci/circleci: py38-core Your tests passed on CircleCI!
Details
ci/circleci: py38-ens Your tests passed on CircleCI!
Details
ci/circleci: py38-ethpm Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-ethtester-pyevm Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-http-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-http-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-ipc-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-ipc-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-ws-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-ws-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-parity-http Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-parity-ipc Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-parity-ws Your tests passed on CircleCI!
Details
continuous-documentation/read-the-docs Read the Docs build succeeded!
Details
@njgheorghita njgheorghita deleted the njgheorghita:type-hints-ethpm branch Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.