Skip to content

gh-131798: Optimize _UNARY_NEGATIVE #135223

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Jun 23, 2025
Merged

gh-131798: Optimize _UNARY_NEGATIVE #135223

merged 10 commits into from
Jun 23, 2025
+33 −2

Conversation

noamcohen97
Copy link
Contributor

@noamcohen97 noamcohen97 commented Jun 6, 2025 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request Jun 6, 2025
Open
Zheaoli
Zheaoli reviewed Jun 6, 2025
View reviewed changes
brandtbucher
brandtbucher approved these changes Jun 6, 2025
View reviewed changes
Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

Looks good, just some suggested improvements. Thanks!

@noamcohen97
Copy link
Contributor Author

Thanks for the review! @brandtbucher @Zheaoli

brandtbucher
brandtbucher approved these changes Jun 7, 2025
View reviewed changes
Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

Awesome! Can you just fix the merge conflict in the test file?

@noamcohen97
Copy link
Contributor Author

Done!

@Fidget-Spinner
Copy link
Member

@noamcohen97 sorry it seems there's merge conflicts again from merging your other PR.

@noamcohen97
Copy link
Contributor Author

@Fidget-Spinner I should have seen this coming 😄

@Fidget-Spinner
Copy link
Member

@noamcohen97 sorry I forgot to merge this in time and seems there's conflicts again 🤦 . Could you please merge in main again? I'll make sure to remember to merge it soon this time.

@Fidget-Spinner Fidget-Spinner self-assigned this Jun 23, 2025
@noamcohen97 noamcohen97 requested a review from tomasr8 as a code owner June 23, 2025 18:46
@noamcohen97 noamcohen97 force-pushed the optimize-unary-negate branch from 72c83d8 to 517d7ef Compare June 23, 2025 18:47
@noamcohen97
Copy link
Contributor Author

@Fidget-Spinner seems like _UNARY_NEGATIVE already got optimized in #135668 without considering float value and non-compact integer values.
I suggest we replace the current implementation with this one. WDYT?

@Fidget-Spinner
Copy link
Member

@noamcohen97 sounds good. Just make sure to use compact ints. Thanks!

Fidget-Spinner
Fidget-Spinner approved these changes Jun 23, 2025
View reviewed changes
Copy link
Member

@Fidget-Spinner Fidget-Spinner left a comment

Choose a reason for hiding this comment

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

Thanks!

@Fidget-Spinner Fidget-Spinner merged commit bda1218 into python:main Jun 23, 2025
55 checks passed
@noamcohen97 noamcohen97 deleted the optimize-unary-negate branch June 23, 2025 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zheaoli Zheaoli Zheaoli left review comments

@tomasr8 tomasr8 tomasr8 approved these changes

@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes

@brandtbucher brandtbucher brandtbucher approved these changes

Assignees

@Fidget-Spinner Fidget-Spinner

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@noamcohen97 @Fidget-Spinner @Zheaoli @tomasr8 @brandtbucher