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

start to use Perflint to improved performance #1063

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

Conversation

07pepa
Copy link
Member

@07pepa 07pepa commented Oct 28, 2024

added option to ruff to guide developers to use more performant constructs in python

@07pepa 07pepa force-pushed the add-perf-linting branch 3 times, most recently from ce4c579 to 559a08d Compare October 28, 2024 21:41
Copy link

@kewldan kewldan left a comment

Choose a reason for hiding this comment

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

TL;DR: Partially useful changes

After looking at all your changes, I conclude that they don't affect performance much unless you're chasing nanoseconds. In general, of course, some changes should really be made to the code base. And some that might reduce execution time by n amount of time are not worth it, as they make the code heavier to read. I would consider to commit partial changes.

IMHO

@CAPITAINMARVEL
Copy link
Contributor

TL;DR: Partially useful changes

After looking at all your changes, I conclude that they don't affect performance much unless you're chasing nanoseconds. In general, of course, some changes should really be made to the code base. And some that might reduce execution time by n amount of time are not worth it, as they make the code heavier to read. I would consider to commit partial changes.

IMHO

like if else expression ? making it more difficult to read ?

@kewldan
Copy link

kewldan commented Nov 2, 2024

TL;DR: Partially useful changes
After looking at all your changes, I conclude that they don't affect performance much unless you're chasing nanoseconds. In general, of course, some changes should really be made to the code base. And some that might reduce execution time by n amount of time are not worth it, as they make the code heavier to read. I would consider to commit partial changes.
IMHO

like if else expression ? making it more difficult to read ?

Not only statements, complicated list/dict comprehension. I didn't say that all changes are useless or over complicated due to reduce execution time, I fully agree with you that some solutions in the current code have some flaws that you want to correct. Don't take it as disrespect 😁

@CAPITAINMARVEL
Copy link
Contributor

TL;DR: Partially useful changes
After looking at all your changes, I conclude that they don't affect performance much unless you're chasing nanoseconds. In general, of course, some changes should really be made to the code base. And some that might reduce execution time by n amount of time are not worth it, as they make the code heavier to read. I would consider to commit partial changes.
IMHO

like if else expression ? making it more difficult to read ?

Not only statements, complicated list/dict comprehension. I didn't say that all changes are useless or over complicated due to reduce execution time, I fully agree with you that some solutions in the current code have some flaws that you want to correct. Don't take it as disrespect 😁

Yeah dont worry, its hard to determine a balance between readability and improving

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.

3 participants