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

Type hint linting #1002

Closed
JustinDrake opened this issue Apr 26, 2019 · 5 comments · Fixed by #1166
Closed

Type hint linting #1002

JustinDrake opened this issue Apr 26, 2019 · 5 comments · Fixed by #1166
Assignees

Comments

@JustinDrake
Copy link
Collaborator

It should be possibly to lint the pyspec type hints. See here:

You can run mypy/a linter on the output pyspec file

@hwwhww
Copy link
Contributor

hwwhww commented Apr 26, 2019

I've tried to add mypy tests - however, the current SSZType + globals() style type definition is quite incompatible with mypy...

I'd say we can solve it with py-ssz. :p

@djrtwo
Copy link
Contributor

djrtwo commented Apr 26, 2019

We can at least mypy and lint the beacon chain pyspec in CI

@hwwhww
Copy link
Contributor

hwwhww commented May 7, 2019

Appy mypy type hinting check with the current spec:

mypy --follow-imports=silent --warn-unused-ignores --ignore-missing-imports --check-untyped-defs --disallow-incomplete-defs --disallow-any-generics -p eth2spec

Result:
https://gist.github.com/hwwhww/d000591903690fe9cc4a5eb8a09ee4a9

As I can see the problem is that the def SSZType + class SSZObject tricks are in runtime:

https://github.com/ethereum/eth2.0-specs/blob/f57d6fa28e151570e9c94807e6729a679fc7b1cf/test_libs/pyspec/eth2spec/utils/minimal_ssz.py#L10-L38

For example:

Fork = SSZType({
    # Previous fork version
    'previous_version': 'bytes4',
    # Current fork version
    'current_version': 'bytes4',
    # Fork epoch number
    'epoch': 'uint64',
})

And the type checker cannot see functions defined at runtime.

On the other hand, py-ssz defines a container as a ssz.sedes.Serializable subclass:

class Fork(ssz.Serializable):

    fields = [
        # Previous fork version
        ('previous_version', bytes4),
        # Current fork version
        ('current_version', bytes4),
        # Fork epoch number
        ('epoch', uint64)
    ]

    def __init__(self,
                 previous_version: bytes,
                 current_version: bytes,
                 epoch: Epoch) -> None:
        super().__init__(
            previous_version=previous_version,
            current_version=current_version,
            epoch=epoch,
        )
  • the fields are knowable to type checkers.
  • The __init__ function rewriting is for type hinting in object initialization.

/cc @protolambda @dankrad

@JustinDrake
Copy link
Collaborator Author

@hwwhww: What is the status of this with the new SSZ typing?

@hwwhww
Copy link
Contributor

hwwhww commented Jun 9, 2019

@JustinDrake I was testing on the early version of the new SSZ, I will start to test on the dev branch now.

@hwwhww hwwhww self-assigned this Jun 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants