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

Better typing hints for make_request #447

Merged

Conversation

nhruo123
Copy link
Contributor

This closes #446, there are 2 issues I faced during the development processes that made this PR larger then expected.

  1. The project currently supports older version of python (3.7 and above). This was an issue due to the fact that I needed to use the ParamSpec type that was introduced in python 3.10. This forced me to create an untyped version of the function and then two definitions depending on the version of python the user is using. This is not ideal, but I don't see a better solution (beside dropping support for older versions but that's way out of the scope of this PR).
  2. The project is using an outdated version of mypy that didn't support the usage of ParamSpec so I had to bump mypy version to 1.0 and above. This introduced some new mypy errors that I had to fix as well.

@nhruo123 nhruo123 marked this pull request as ready for review July 30, 2024 10:03
@nhruo123 nhruo123 marked this pull request as draft July 30, 2024 10:17
@nhruo123 nhruo123 marked this pull request as ready for review July 30, 2024 10:20
@nhruo123
Copy link
Contributor Author

I can't make the integration test run on my machine, please let me know if I broke any

@michaelhly
Copy link
Owner

@nhruo123 are you able to run make lint? Seem like mypy is complaining:

src/solana/rpc/async_api.py:1108: error: Value of type variable "T" of "make_request" of "AsyncHTTPProvider" cannot be "ValidatorExitResp"  [type-var]
src/solana/rpc/api.py:1103: error: Value of type variable "T" of "make_request" of "HTTPProvider" cannot be "ValidatorExitResp"  [type-var]

@nhruo123
Copy link
Contributor Author

@nhruo123 are you able to run make lint? Seem like mypy is complaining:

src/solana/rpc/async_api.py:1108: error: Value of type variable "T" of "make_request" of "AsyncHTTPProvider" cannot be "ValidatorExitResp"  [type-var]
src/solana/rpc/api.py:1103: error: Value of type variable "T" of "make_request" of "HTTPProvider" cannot be "ValidatorExitResp"  [type-var]

My bad, I didn't stage that change. Yea, this is a slight issue as ValidatorExitResp is not part of RPCResult (which T is bound to), and we have no control over that type because its in Solders. For now I just added a a comment to ignore typing on the relevant lines, but a long term solution should be to either create our own type over RPCResult and ValidatorExitResp or try to push for a change in Solders.

@michaelhly michaelhly merged commit 8b97153 into michaelhly:master Jul 30, 2024
11 of 12 checks passed
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.

Better type hinting for make_request on AsyncHTTPProvider
2 participants