-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(types): update type hints in web3._utils.method_formatters.py #3669
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
base: main
Are you sure you want to change the base?
Conversation
web3/_utils/method_formatters.py
Outdated
@@ -1214,7 +1211,7 @@ def get_result_formatters( | |||
|
|||
def get_error_formatters( | |||
method_name: Union[RPCEndpoint, Callable[..., RPCEndpoint]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is method_name actually supposed to be a Union[RPCEndpoint, Callable[..., RPCEndpoint]]
?
I notice Union[RPCEndpoint, Callable[..., RPCEndpoint]]
is used throughout the codebase but the code itself seems to assume method_name is always a string value, never callable. Should this be updated as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, nice catch. Yeah, these can only be RPCEndpoints once they're here.
06680f0
to
6ec8bed
Compare
6ec8bed
to
d8f7ccf
Compare
@@ -1190,7 +1187,7 @@ def filter_wrapper( | |||
|
|||
@to_tuple | |||
def apply_module_to_formatters( | |||
formatters: Tuple[Callable[..., TReturn]], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious about the reasoning behind this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change just indicates to mypy that the function will work with any iterable of callables.
The original hint indicates that the function must take a tuple input, so while it didn't cause any type errors internally here it can cause unneeded type errors for downstream users.
web3/_utils/method_formatters.py
Outdated
@@ -1214,7 +1211,7 @@ def get_result_formatters( | |||
|
|||
def get_error_formatters( | |||
method_name: Union[RPCEndpoint, Callable[..., RPCEndpoint]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, nice catch. Yeah, these can only be RPCEndpoints once they're here.
What was wrong?
get_result_formatters
had return typeDict[str, Callable[..., Any]]
which is incorrect because the function returns a callabletoolz.compose
objectget_error_formatters
, andget_null_result_formatters
both had return typeCallable[..., Any]
which is techincally correct but could be made more specific for clarity for contributors.Related to Issue #N/A
Closes #N/A
How was it fixed?
I've changed the return type for all 3 functions to
Callable[[RPCResponse], Any]
.Todo:
I did not add an entry to release notes as this change is not relevant for users.
Cute Animal Picture