-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-118767: Make bool(NotImplemented) raise TypeError #118775
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
Changes from all commits
8428e3c
ee8492f
df6a76a
0e83fd0
5daaa9d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -174,6 +174,9 @@ for more details. | |
it currently evaluates as true, it will emit a :exc:`DeprecationWarning`. | ||
It will raise a :exc:`TypeError` in a future version of Python. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be removed. |
||
|
||
.. versionchanged:: 3.14 | ||
Evaluating :data:`NotImplemented` in a boolean context now raises a :exc:`TypeError`. | ||
|
||
|
||
Ellipsis | ||
-------- | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2130,14 +2130,11 @@ def test_warning_notimplemented(self): | |
# be evaluated in a boolean context (virtually all such use cases | ||
# are a result of accidental misuse implementing rich comparison | ||
# operations in terms of one another). | ||
# For the time being, it will continue to evaluate as a true value, but | ||
# issue a deprecation warning (with the eventual intent to make it | ||
# a TypeError). | ||
self.assertWarns(DeprecationWarning, bool, NotImplemented) | ||
with self.assertWarns(DeprecationWarning): | ||
self.assertRaises(TypeError, bool, NotImplemented) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test should be renamed. |
||
with self.assertRaises(TypeError): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TypeError is raised when you just call a function with wrong number of arguments, for example: with self.assertRaises(TypeError):
self.assertEqual(NotImplemented) It is better to not use if NotImplemented:
pass There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll also change it to use |
||
self.assertTrue(NotImplemented) | ||
with self.assertWarns(DeprecationWarning): | ||
self.assertFalse(not NotImplemented) | ||
with self.assertRaises(TypeError): | ||
not NotImplemented | ||
|
||
|
||
class TestBreakpoint(unittest.TestCase): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
Using :data:`NotImplemented` in a boolean context now raises | ||
:exc:`TypeError`. Contributed by Jelle Zijlstra in :gh:`118767`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't the link to the issue already included in the changelog? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, fixed in the new PR. |
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.
This can be removed. And maybe the whole text of the note can be reworded.
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, I'll submit another PR addressing your comments.