Skip to content

bpo-41361: Converting collections.deque methods to Argument Clinic #21584

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

Closed
wants to merge 1 commit into from

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Jul 21, 2020

@rhettinger
Copy link
Contributor

Please don't change all the variable names inside the functions — that churns lots of code unnecessarily and it makes it more difficult to see what changed.

Also, is there any advantage to doing all this? From my point of view, it greatly clutters the code for very little benefit. It also makes it more difficult to see whether the calling patterns are optimized.

@rhettinger
Copy link
Contributor

I've thought about this more and think we should leave the code as-is. All of these changes interfere with my ability to read and maintain the code. Without a compelling benefit, we should just skip it. We already applied vectorcall in the place where it would help. If we really thought it necessary, we could use text signatures, but most of these methods already have very simple signatures.

@rhettinger rhettinger closed this Jul 22, 2020
@rhettinger
Copy link
Contributor

Thank you for your efforts and your understanding :-)

@corona10
Copy link
Member Author

Thank you for your efforts and your understanding :-)

Thank you for your explanation ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants