Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(207837)

#27814: contextlib.suppress: Add whitelist argument

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 3 months ago by m
Modified:
3 years, 3 months ago
Reviewers:
ghost.adh
CC:
barry, rhettinger, Nick Coghlan, SilentGhost, Yury Selivanov, mb_
Visibility:
Public.

Patch Set 1 #

Total comments: 1

Patch Set 2 #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/contextlib.rst View 1 4 chunks +14 lines, -2 lines 4 comments Download
Lib/contextlib.py View 1 2 chunks +32 lines, -2 lines 0 comments Download
Lib/test/test_contextlib.py View 1 1 chunk +25 lines, -0 lines 2 comments Download

Messages

Total messages: 2
mb_
https://bugs.python.org/review/27814/diff/18187/Lib/contextlib.py File Lib/contextlib.py (right): https://bugs.python.org/review/27814/diff/18187/Lib/contextlib.py#newcode259 Lib/contextlib.py:259: except PermissionError: There's a copy&paste error here. This should ...
3 years, 3 months ago #1
SilentGhost
3 years, 3 months ago #2
https://bugs.python.org/review/27814/diff/18188/Doc/library/contextlib.rst
File Doc/library/contextlib.rst (right):

https://bugs.python.org/review/27814/diff/18188/Doc/library/contextlib.rst#ne...
Doc/library/contextlib.rst:121: A whitelist of exceptions that will not
suppressed can also be specified
"will not be"

https://bugs.python.org/review/27814/diff/18188/Doc/library/contextlib.rst#ne...
Doc/library/contextlib.rst:122: as one exception or a tuple of exceptions to the
*unless* keyword
I would say "via" rather than "to".

https://bugs.python.org/review/27814/diff/18188/Doc/library/contextlib.rst#ne...
Doc/library/contextlib.rst:124: Any whitelisted exception will not be
suppressed.
This just seems to repeat the point above.

https://bugs.python.org/review/27814/diff/18188/Doc/library/contextlib.rst#ne...
Doc/library/contextlib.rst:126: of the suppressed exceptions for this operation
to make sense. But that
Double space between sentences is needed.

https://bugs.python.org/review/27814/diff/18188/Lib/test/test_contextlib.py
File Lib/test/test_contextlib.py (right):

https://bugs.python.org/review/27814/diff/18188/Lib/test/test_contextlib.py#n...
Lib/test/test_contextlib.py:959: 1/0
Wouldn't this be clearer if you were to simply raise the desired exception? I
see that that's how it's done in other test in this class but I for the life of
me cannot see why.

https://bugs.python.org/review/27814/diff/18188/Lib/test/test_contextlib.py#n...
Lib/test/test_contextlib.py:977: with suppress(Exception, unless=(IndexError,
OSError)):
This is essentially previous test, perhaps testing multiple exception would be
more useful? e.g.:

with suppress(ArithmeticError, OSError, unless=(FloatingPointError,
FileExistsError)):
    raise ZeroDivisionError
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+