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

Change uint aliases to just be subclasses #1216

Merged
merged 3 commits into from Jun 25, 2019
Merged

Change uint aliases to just be subclasses #1216

merged 3 commits into from Jun 25, 2019

Conversation

protolambda
Copy link
Collaborator

Change uint aliases to just be subclasses do not override init with no-op.

old (note that the ellipsis here is not for code showing purposes, it's python code):

class ValidatorIndex(uint64):
    def __init__(self, _x: uint64) -> None:
        ...

new:

class ValidatorIndex(uint64):
    pass

The subclassing works just fine, and does not have all the edge-cases to think about when overriding the __init__. Also enables (read: makes IDE type hint checker happy) us to do ValidatorIndex(123) instead of ValidatorIndex(uint64(123)).

Also experimented with the use of __metaclass__ or Elements.elem_type, ElementsType.elem_type in type hinting. Intellij + Mypy are happy (surprisingly), but Python is not. So we would have to shadow the bahavior of Generic to do that kind of type-hinting. That is for another time, construction of types and passing them is the most important now. The getter/setter/iteration is not even picked up as a problem by mypy, while it could be better (if Python allowed us to).

And fixed an old validator_registry reference, which was missed (genesis trigger testing is in progress I believe)

@protolambda protolambda added the general:presentation Presentation (as opposed to content) label Jun 25, 2019
@protolambda protolambda requested a review from hwwhww June 25, 2019 18:57
@djrtwo djrtwo merged commit ab012b8 into dev Jun 25, 2019
@protolambda protolambda deleted the fix-type-aliasing branch February 9, 2020 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general:presentation Presentation (as opposed to content)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants