Remove Python 2 support, add types #7
Conversation
This file contains hidden or 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
Python 2 is deprecated.
This isn't required in Python 3.
We no longer support Python 2, it's over.
It makes it easier to declare the type signatures without having to resort to using strings since the class is defined later.
Python 2 is well and truly gone.
We still have a variety of typing errors, but this at least gives us something to target based on the behavior of the unit tests.
I don't know if this is mathematically correct, but I'm not sure why the code is returning a new Inf rather than self. I'll leave it for now.
We manually specify the expected types for addend and result since they may take either Point or Inf. Also de-indent by raising an exception earlien for clarity.
This one is a bit tricky. We need to provide clear paths where the mypy engine can eliminate the potential to have None types. This means creating a somewhat clunky conditional for setting up the member variables and raising exceptions in a few different places.
This requires explicitly checking for None so that mypy can detect that the Optional has been resolved and we no longer have uncertain types. This does somewhat damage the readability of X.can_sign, so the double check is left in at a slight performance penalty. I also renamed the "keypair" parameter as I find it more clearly readable instead of having to realize that self.keypair and keypair are different. Also avoid the Union type in return by catching and raising on the Inf case. This may not be strictly correct.
Without this we may have an Inf type, which we really shouldn't ever get.
The big change here is that we now make the known curves into typed data structures instead of just nested mappings. This represents a change in the public API.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Python 2 was sunset in 2020, so it's worthwhile to remove compatibility shims now.
Type hinting information is quite helpful for anyone hoping to learn by reading the code.