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 type hints to web3.providers.ethtester #1518

Merged

Conversation

@njgheorghita
Copy link
Contributor

njgheorghita commented Nov 20, 2019

What was wrong?

Type hints missing from web3.providers.ethtester

Cute Animal Picture

image

@njgheorghita njgheorghita force-pushed the njgheorghita:type-hints-providers-ethtester branch 2 times, most recently from 9648037 to 48fcf7a Nov 20, 2019
@njgheorghita njgheorghita requested a review from pipermerriam Nov 20, 2019
Copy link
Member

pipermerriam left a comment

Would like to re-review once you've made another pass at tightening some of the types (assuming they can actually be tightened)

@@ -190,7 +191,7 @@ def address_in(address: ChecksumAddress, addresses: Collection[ChecksumAddress])


def address_to_reverse_domain(address: ChecksumAddress) -> str:
lower_unprefixed_address = remove_0x_prefix(to_normalized_address(address))
lower_unprefixed_address = remove_0x_prefix(HexStr(to_normalized_address(address)))

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Nov 20, 2019

Member

Not for this PR but it would be ideal if we got our typing right so that we didn't need to wrap this in HexStr.

if fn_kwargs is None:
fn_kwargs = {}
return getattr(eth_tester, fn_name)(*fn_args, **fn_kwargs)


def without_eth_tester(fn):
def without_eth_tester(fn: Callable[..., Any]) -> Callable[..., RPCResponse]:

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Nov 20, 2019

Member

I think this would be more accurately typed as:

TReturn = TypeVar("TReturn")
TParams = TypeVar("TParams")

def without_eth_tester(fn: Callable[[TParams], TReturn]) -> Callable[[EThereumTester, TParams], TReturn]:
    ...
return fn(params)
return inner


def without_params(fn):
def without_params(fn: Callable[..., Any]) -> Callable[..., RPCResponse]:

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Nov 20, 2019

Member

Same here. Typing can be more precicesly defined using TypeVar types.

return eth_tester, preprocessor_fn(params)


def static_return(value):
def inner(*args, **kwargs):
def static_return(value: Any) -> Callable[..., Any]:

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Nov 20, 2019

Member

This one should be doable as def static_return(value: TValue) -> Callable[..., TValue]:

@@ -73,7 +101,7 @@ def client_version(eth_tester, params):


@curry
def null_if_excepts(exc_type, fn):
def null_if_excepts(exc_type: Type[BaseException], fn: Callable[..., Any]) -> Callable[..., Any]:

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Nov 20, 2019

Member

Same here, types can be tightened using TypeVar for callable return type.

filter_params = params[0]
filter_id = eth_tester.create_log_filter(**filter_params)
return filter_id


def get_logs(eth_tester, params):
def get_logs(eth_tester: "EthereumTester", params: Any) -> List[LogReceipt]:

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Nov 20, 2019

Member

For each of these that is a defined API we should know what the params value looks like and thus be able to use stricture types than Any.

This comment has been minimized.

Copy link
@njgheorghita

njgheorghita Nov 21, 2019

Author Contributor

👍 Will take care of tightening up the params value in a different pr to avoid bloat.

return eth_tester.add_account(_generate_random_private_key())


def personal_send_transaction(eth_tester, params):
def personal_send_transaction(eth_tester: "EthereumTester", params: Any) -> HexStr:

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Nov 20, 2019

Member

params here should be someting like Optional[TxParams] I think.

async def make_request(self, method, params):
async def make_request(
self, method: RPCEndpoint, params: Any
) -> Coroutine[Any, Any, RPCResponse]:

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Nov 20, 2019

Member

I'm confused by this returning a Coroutine type. I would have expected just RPCResponse...

@@ -55,7 +81,7 @@ def __init__(self, ethereum_tester=None, api_endpoints=None):
else:
self.api_endpoints = api_endpoints

def make_request(self, method, params):
def make_request(self, method: RPCEndpoint, params: Any) -> RPCResponse:

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Nov 20, 2019

Member

At minimum do we know that params is a List[Any]? I'm not 100% sure but I think that we have some knowledge that params is a list of values.

This comment has been minimized.

Copy link
@njgheorghita

njgheorghita Nov 21, 2019

Author Contributor

params This will be a list or other iterable of the parameters for the JSON-RPC method being called.

Yup! - but it from some testing it seems that it requires a List type not Sequence / Iterable.

This comment has been minimized.

Copy link
@njgheorghita

njgheorghita Nov 21, 2019

Author Contributor

Making this update touches a lot of files, so to avoid from bloating this pr, i'll follow it up with another that tightens this up

@njgheorghita njgheorghita force-pushed the njgheorghita:type-hints-providers-ethtester branch from 48fcf7a to e043637 Nov 21, 2019
@njgheorghita njgheorghita requested a review from pipermerriam Nov 21, 2019
@njgheorghita njgheorghita merged commit d0ae459 into ethereum:master Nov 22, 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-providers-ethtester branch Nov 22, 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.