-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
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.
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
Conversation
|
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 |
Misc/NEWS.d/next/Library/2024-02-22-10-12-59.gh-issue-115808.F2g2Ku.rst
Outdated
Show resolved
Hide resolved
|
Please, update the documentation for this module (Doc/library/operator.rst) |
|
@Eclips4 Thank you for the suggestion, updated it accordingly. |
AlexWaygood
left a comment
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.
Thanks! In order for this to be merged, it would also need:
- Documentation to be added in
Doc/library/operator.rst - An entry in
Doc/whatsnew/3.13.rst - An additional implementation written in C in https://github.com/python/cpython/blob/main/Modules/_operator.c
Here's the most recent PR that added a new function to the operator module, which you could use as a reference:
|
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 have made the requested changes; please review again |
AlexWaygood
left a comment
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.
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.
|
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
left a comment
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.
Thank you! I have several nitpicks :)
Eclips4
left a comment
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.
Looks very good for me. Good job @ThexXTURBOXx 👍
bateller
left a comment
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.
👍
434bc8b to
354acfb
Compare
|
Rebased on |
|
Rebased on |
|
Rebased on |
|
Thanks for the comments - fixed! |
Looks good, thanks. However, what is about 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_none and is_not_none operators
|
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
left a comment
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. Thanks @ThexXTURBOXx for your efforts!
|
You're more than welcome, thanks for the reviews! :) |
|
Thanks all! A |
…ython#115814) Co-authored-by: Kirill Podoprigora <[email protected]>
This PR adds an
is_noneoperator and an appropriate test.I hope I did not forget to update/reference it anywhere else.
Fixes #115808