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

refactor: clean up various refactorings #6165

Closed
wants to merge 4 commits into from
Closed

refactor: clean up various refactorings #6165

wants to merge 4 commits into from

Conversation

kkirsche
Copy link
Contributor

  1. Use contextlib.suppress rather than empty except
  2. Prefer f-strings to .format and +-based concatenation for improved performance.
  3. Remove empty elif via refactoring.
  4. Use yield from instead of yield for 15–20% performance gain (on average)
  5. Generate list() rather than iteration
  6. Prefer generators to looped iteration

@kkirsche
Copy link
Contributor Author

Oops, accidentally applied a named expression. I'll fix that

@sigmavirus24
Copy link
Contributor

Thanks for the effort but we do not accept arbitrary refactorings of numerous disparate places in the code that aren't related. This tends to invite floods if other questionable large scale refactorings. The refactoring is usually largely subjective and unsolicited.

@kkirsche kkirsche deleted the refactor/f-strings branch June 17, 2022 19:41
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants