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
Wrong and confusing type annotations #318
Labels
bug
Something isn't working
Comments
Thanks for notifying me. These annotations used to be valid, as CI has been running passing typechecking for a while. However, recently typecheckers like mypy (not only pylance) have started enforcing no-implicit-optional, which I remedied in this PR along with other typing issues reported with newer mypy: #320 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The wrong type annotation was used in the
uniswap.uniswap.Uniswap
class. The parametersprovider
,web3
,factory_contract_addr
androuter_contract_addr
of the constructor default toNone
but are not declared asOptional
. This is very unfriendly to static type checkers (such as Pyright with strict type checking enabled). And it will generate a lot of type error prompts, but actually no problem.At the same time, the type annotation format of
address
andprivate_key
is confusing. In Python3.9 and below, the recommended declaration method isOptional[T]
to indicate that the parameter can beNone
. And in 3.10+, its recommended to useT | None
(alsoUnion[T, None]
). But here we can see the two different formats of type annotations are being used at the same time, which is confusing.I suggest changing it to the following:
Also, I noticed that some function parameters/return values also have such annotations. Returns possible
None
but noOptional
is declared. This will confuse the user for wasting more time debugging the type.The text was updated successfully, but these errors were encountered: