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

#27066: SystemError if custom opener returns -1

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 8 months ago by barry
Modified:
3 years, 8 months ago
Reviewers:
storchaka+cpython, vadmium+py
CC:
barry, devnull_psf.upfronthosting.co.za, storchaka, ppperry
Visibility:
Public.

Patch Set 1 #

Total comments: 4

Patch Set 2 #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/test/test_io.py View 1 1 chunk +16 lines, -0 lines 2 comments Download
Modules/_io/fileio.c View 1 1 chunk +5 lines, -1 line 4 comments Download

Messages

Total messages: 5
storchaka
https://bugs.python.org/review/27066/diff/17284/Lib/test/test_io.py File Lib/test/test_io.py (right): https://bugs.python.org/review/27066/diff/17284/Lib/test/test_io.py#newcode788 Lib/test/test_io.py:788: self.assertRaises(OSError, open, 'non-existent', 'r', Since this assertRaises() takes two ...
3 years, 8 months ago #1
barry
Thanks for the review. I'll attach an updated diff to the issue. https://bugs.python.org/review/27066/diff/17284/Lib/test/test_io.py File Lib/test/test_io.py ...
3 years, 8 months ago #2
storchaka
http://bugs.python.org/review/27066/diff/17324/Lib/test/test_io.py File Lib/test/test_io.py (right): http://bugs.python.org/review/27066/diff/17324/Lib/test/test_io.py#newcode790 Lib/test/test_io.py:790: self.assertEqual(str(cm.exception), 'opener returned -1') Or you can just use ...
3 years, 8 months ago #3
Martin Panter
https://bugs.python.org/review/27066/diff/17324/Modules/_io/fileio.c File Modules/_io/fileio.c (right): https://bugs.python.org/review/27066/diff/17324/Modules/_io/fileio.c#newcode425 Modules/_io/fileio.c:425: /* The opener returned -1. See issue #27066 */ ...
3 years, 8 months ago #4
barry
3 years, 8 months ago #5
https://bugs.python.org/review/27066/diff/17324/Lib/test/test_io.py
File Lib/test/test_io.py (right):

https://bugs.python.org/review/27066/diff/17324/Lib/test/test_io.py#newcode790
Lib/test/test_io.py:790: self.assertEqual(str(cm.exception), 'opener returned
-1')
On 2016/05/23 21:32:02, storchaka wrote:
> Or you can just use assertRaisesRegex.

I could, but I think I like it more explicit this way.  Thanks for the
suggestion though!

https://bugs.python.org/review/27066/diff/17324/Modules/_io/fileio.c
File Modules/_io/fileio.c (right):

https://bugs.python.org/review/27066/diff/17324/Modules/_io/fileio.c#newcode424
Modules/_io/fileio.c:424: if (!PyErr_Occurred())
On 2016/05/23 21:32:02, storchaka wrote:
> I think that braces are not only requirement of PEP 7, but increases
> readability, since the body exists of two "statements": the comment and
> PyErr_Format().

Braces are only "strongly preferred" in PEP 7 where C doesn't require them. 
Still, you might be right that it improves readability in this case.

https://bugs.python.org/review/27066/diff/17324/Modules/_io/fileio.c#newcode425
Modules/_io/fileio.c:425: /* The opener returned -1.  See issue #27066 */
Thanks.  I'll improve the comment in my git repo.  I won't submit a new patch
but I'll apply it after we switch to git.
Sign in to reply to this message.

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