-
Notifications
You must be signed in to change notification settings - Fork 1
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
Switch from flake8/yapf to ruff. #65
Conversation
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.
LGTM.
self.run(self.benchmark_steps) | ||
performance.append(self.get_performance()) | ||
if print_verbose_messages: | ||
print(f'.. {performance[-1]} {self.units}') | ||
else: | ||
for i in range(self.repeat): | ||
for _i in range(self.repeat): |
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.
Did ruff
make this change? If so, should we let a formatter change variable names?
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.
Yes, it was one of the unsafe fixes that I explicitly opted into on the command line --unsafe-fixes
and then carefully reviewed (https://docs.astral.sh/ruff/rules/unused-loop-control-variable/). Running pre-commit run
will not make these changes.
This particular change solves https://docs.astral.sh/ruff/rules/unused-loop-control-variable/. I don't particularly agree with underscore prefixes as a solution to solve unused variables, but when switching to standard tools we are opting into what is considered the "standard" solution.
Description
Switch from flake8/yapf to ruff.
Motivation and context
Perform more lint checks and format in less time.
How has this been tested?
Local execution:
Checklist:
CONTRIBUTORS.md
) in the pull request source branch.