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 annotations #19

Merged
merged 1 commit into from
Nov 20, 2017
Merged

Conversation

gsalgado
Copy link
Collaborator

This branch includes a pytest fixture that uses pyannotate to dump a json file with type information.

Once you feed that to pyannotate it will generate the type annotation comments seen in the diff here, although I had to modify some of them for various reasons (that is expected).

With those comments we can use mypy to perform static type checking, like this:

$ mypy --py2 eth_keys/main.py

@pipermerriam
Copy link
Member

Big 👍 from me on this.

Is it python2 that restricts us from using the annotation format found here? https://docs.python.org/3/library/typing.html

@gsalgado
Copy link
Collaborator Author

Mainly that, yes, but pyannotate does not support py3-style annotations yet so we'd have to add them all manually if we wanted to go that way. Are you planning to drop 2.7 support here as well?

@gsalgado gsalgado force-pushed the type-annotations branch 11 times, most recently from 445f149 to 2b8374a Compare November 20, 2017 14:41
@gsalgado gsalgado changed the title WIP: Add type annotations Add type annotations Nov 20, 2017
@gsalgado
Copy link
Collaborator Author

@pipermerriam I don't seem to have rights to ask for a review on this repo, so doing that by pinging you here

@pipermerriam
Copy link
Member

Are you planning to drop 2.7 support here as well?

Yes, my plan is to drop 2.7 support across all of the tooling I maintain by the end of Q1 2018.

@@ -1,6 +1,7 @@
from __future__ import absolute_import

import os
from typing import Optional # noqa: F401
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming this is due to the use of this within the type annotation comments. Odd but also makes sense.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, yes

@@ -3,6 +3,7 @@
"""
import hashlib
import hmac
from typing import (Callable, Optional, Tuple, Union) # noqa: F401
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Union doesn't appear to be used here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, removed

Also adds a new tox testenv that uses mypy to perform static type checks
@gsalgado gsalgado merged commit d9958a8 into ethereum:master Nov 20, 2017
pacrob pushed a commit to pacrob/eth-keys that referenced this pull request Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants