Skip to content

gh-115808: Add is_none and is_not_none operators #115814

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 14 commits into from
Aug 10, 2024
Merged

gh-115808: Add is_none and is_not_none operators #115814

merged 14 commits into from
Aug 10, 2024

Conversation

ThexXTURBOXx
Copy link
Contributor

@ThexXTURBOXx ThexXTURBOXx commented Feb 22, 2024 •
edited by AlexWaygood
Loading

This PR adds an is_none operator and an appropriate test.
I hope I did not forget to update/reference it anywhere else.

Fixes #115808

@ghost
Copy link

ghost commented Feb 22, 2024 •
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app bedevere-app bot mentioned this pull request Feb 22, 2024
Closed
@bedevere-app
Copy link

bedevere-app bot commented Feb 22, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@Eclips4 Eclips4 requested a review from rhettinger February 22, 2024 10:17
@Eclips4
Copy link
Member

Eclips4 commented Feb 22, 2024

Please, update the documentation for this module (Doc/library/operator.rst)

@ThexXTURBOXx
Copy link
Contributor Author

@Eclips4 Thank you for the suggestion, updated it accordingly.

AlexWaygood
AlexWaygood previously requested changes Feb 22, 2024
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks! In order for this to be merged, it would also need:

Here's the most recent PR that added a new function to the operator module, which you could use as a reference:

@bedevere-app
Copy link

bedevere-app bot commented Feb 22, 2024

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@ThexXTURBOXx
Copy link
Contributor Author

I have made the requested changes; please review again

AlexWaygood
AlexWaygood reviewed Feb 22, 2024
View reviewed changes
@ThexXTURBOXx
Copy link
Contributor Author

I have made the requested changes; please review again

@AlexWaygood AlexWaygood dismissed their stale review February 22, 2024 14:49

Requested changes were made

@AlexWaygood AlexWaygood requested a review from Eclips4 February 22, 2024 15:09
AlexWaygood
AlexWaygood reviewed Feb 22, 2024
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks! This looks pretty good to me now. We'll need to wait a while to see if anybody voices any objections on the issue before we can consider merging this, though.

@ThexXTURBOXx
Copy link
Contributor Author

No problem, take your time! And thank you very much for your valuable comments. They helped me shape my first PR here such that it fits your needs!

sobolevn
sobolevn reviewed Feb 22, 2024
View reviewed changes
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thank you! I have several nitpicks :)

@ThexXTURBOXx ThexXTURBOXx mentioned this pull request Feb 22, 2024
Closed
Eclips4
Eclips4 approved these changes Feb 22, 2024
View reviewed changes
Copy link
Member

@Eclips4 Eclips4 left a comment

Choose a reason for hiding this comment

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

Looks very good for me. Good job @ThexXTURBOXx 👍

bateller
bateller approved these changes Feb 22, 2024
View reviewed changes
Copy link

@bateller bateller left a comment

Choose a reason for hiding this comment

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

👍

@ThexXTURBOXx
Copy link
Contributor Author

ThexXTURBOXx commented Mar 4, 2024 •
edited by Eclips4
Loading

Rebased on main

@ThexXTURBOXx
Copy link
Contributor Author

Rebased on main

@ThexXTURBOXx
Copy link
Contributor Author

Rebased on main

Eclips4
Eclips4 reviewed Aug 6, 2024
View reviewed changes
@ThexXTURBOXx
Copy link
Contributor Author

Thanks for the comments - fixed!

@Eclips4
Copy link
Member

Eclips4 commented Aug 6, 2024

Thanks for the comments - fixed!

Looks good, thanks.

However, what is about is_not_none #115808 (comment)?

I think it should be implemented, and I think that it is okay to do so in this PR, since these things are related.

cc @AlexWaygood @rhettinger

@ThexXTURBOXx
Copy link
Contributor Author

However, what is about is_not_none #115808 (comment)?
I think it should be implemented, and I think that it is okay to do so in this PR, since these things are related.

You are right. I haven't seen this comment yet. I have also added an appropriate is_not_none function.

Eclips4
Eclips4 reviewed Aug 6, 2024
View reviewed changes
@Eclips4 Eclips4 changed the title gh-115808: Add is_none operator gh-115808: Add is_none and is_not_none operators Aug 6, 2024
@AA-Turner
Copy link
Member

If there are no outstanding concerns I'll look to merge this in the next few days (cc @Eclips4 if you have any final comments)

A

Eclips4
Eclips4 approved these changes Aug 7, 2024
View reviewed changes
Copy link
Member

@Eclips4 Eclips4 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @ThexXTURBOXx for your efforts!

@ThexXTURBOXx
Copy link
Contributor Author

You're more than welcome, thanks for the reviews! :)

@AA-Turner AA-Turner merged commit 5580f31 into python:main Aug 10, 2024
39 checks passed
@AA-Turner
Copy link
Member

Thanks all!

A

@ZeroIntensity ZeroIntensity mentioned this pull request Aug 17, 2024
Merged
blhsing pushed a commit to blhsing/cpython that referenced this pull request Aug 22, 2024
@ThexXTURBOXx @Eclips4 @blhsing
pythongh-115808: Add is_none and is_not_none to operator (p…
34f25eb 
…ython#115814)

Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sobolevn sobolevn sobolevn left review comments

@Fidget-Spinner Fidget-Spinner Fidget-Spinner left review comments

@AlexWaygood AlexWaygood AlexWaygood left review comments

@bateller bateller bateller approved these changes

@AA-Turner AA-Turner AA-Turner approved these changes

@Eclips4 Eclips4 Eclips4 approved these changes

@rhettinger rhettinger Awaiting requested review from rhettinger

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add operator.is_none()
7 participants
@ThexXTURBOXx @Eclips4 @AA-Turner @sobolevn @bateller @Fidget-Spinner @AlexWaygood