Skip to content

chore: refactor out _apply_request_formatters #3670

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

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

BobTheBuidler
Copy link
Contributor

@BobTheBuidler BobTheBuidler commented Apr 17, 2025

What was wrong?

Nothing was wrong, but I noticed _apply_request_formatters is a private function only used in 1 place in the codebase, and not particularly efficient due to its unnecessary use of toolz.pipe so I refactored it out completely.

Related to Issue #N/A
Closes #N/A

How was it fixed?

replaced with a simple if check

Todo:

  • Clean up commit history
  • Add or update documentation related to these changes

I did not add an entry to release notes as this change is not relevant for a typical user.

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

if self.json_rpc_method in RPC_METHODS_UNSUPPORTED_DURING_BATCH:
raise MethodNotSupported(
f"Method `{self.json_rpc_method}` is not supported within a batch "
"request."
)
return module.retrieve_request_information(self)
return module.retrieve_request_information(self) # type: ignore [return-value]
Copy link
Contributor Author

@BobTheBuidler BobTheBuidler May 14, 2025

Choose a reason for hiding this comment

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

not really sure how to deal with the line len err here

@BobTheBuidler
Copy link
Contributor Author

This went beyond its original scope, I will break it up into smaller PRs and rebase it as they merge.

else formatted_dict
)
formatted: Dict[str, Any]
formatted = apply_formatters_to_dict(formatters, value) # type: ignore [arg-type]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

had to move this due to line len err

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.

1 participant