This repository has been archived by the owner on Jul 1, 2021. It is now read-only.
Update mypy and fix type hints #506
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pipermerriam
force-pushed
the
piper/update-eth-utils
branch
from
8fc8f50
to
598b531
Compare
pipermerriam
force-pushed
the
piper/update-eth-utils
branch
from
598b531
to
fdec4fa
Compare
carver
approved these changes
Apr 17, 2019
| def shuffle(values: Sequence[TItem], | ||
| seed: Hash32, | ||
| shuffle_round_count: int) -> Iterable[TItem]: | ||
| shuffle_round_count: int) -> Tuple[TItem, ...]: | ||
| return tuple(_shuffle(values, seed, shuffle_round_count)) |
There was a problem hiding this comment.
Is this needed because of the weird @to_tuple mypy bug in we're seeing in eth-utils?
There was a problem hiding this comment.
Yes, this one. I'll update with a code comment: ethereum/eth-utils#152
| @@ -60,7 +59,7 @@ def configure_parser(self, arg_parser: ArgumentParser, subparser: _SubParsersAct | |||
| help="Disables the JSON-RPC Server", | |||
| ) | |||
|
|
|||
| def setup_eth1_modules(self, trinity_config: TrinityConfig) -> Tuple[Eth1ChainRPCModule, ...]: | |||
| def setup_eth1_modules(self, trinity_config: TrinityConfig) -> Tuple[BaseRPCModule, ...]: | |||
There was a problem hiding this comment.
Since these methods are specifically returning this subset of modules, can't/shouldn't they be Eth1... modules? Same for beacon below.
There was a problem hiding this comment.
I don't actually know. I'll spend a bit more time understanding why we have the two types.
There was a problem hiding this comment.
looking at it, it doesn't appear to matter. Each of these functions return values are fed into RPCServer which only cares about getting an iterable of BaseRPCModule.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What was wrong?
eth-utilsfunctions that I want to use that were ported from the trinity codebase.mypyversion that is faster and fixes some thingseth-utilsfor preserving type safety when usingto_dictorto_ordered_dictHow was it fixed?
Updated the two dependencies and addressed the various typing issues that
mypyyelled about.Cute Animal Picture