classification
Title: PEP 3151 implementation
Type: enhancement Stage: committed/rejected
Components: Interpreter Core, Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: pitrou Nosy List: Arfrever, Trundle, barry, djc, eric.araujo, nadeem.vawda, ncoghlan, petri.lehtinen, pitrou, python-dev, rpointel, skrah
Priority: release blocker Keywords: patch

Created on 2011-07-13 22:19 by pitrou, last changed 2011-12-15 13:34 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
pep3151-1.diff pitrou, 2011-07-13 23:40 review
pep3151-2.diff pitrou, 2011-07-21 22:31 review
pep3151-3.diff pitrou, 2011-08-16 18:06 review
pep3151-4.diff pitrou, 2011-08-20 19:22 review
pep3151-5.diff pitrou, 2011-08-30 16:57 review
pep3151-6.diff pitrou, 2011-10-03 18:31 review
oserror_new.patch pitrou, 2011-12-15 09:47 review
oserror_new2.patch pitrou, 2011-12-15 13:06 review
Repositories containing patches
http://hg.python.org/features/pep-3151/#pep-3151
Messages (22)
msg140314 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-07-13 22:19
I now have a working PEP 3151 implementation, working on various platforms (tested on Linux, Windows, FreeBSD and OpenIndiana buildbots).

The implementation has all the semantic changes and additions in PEP 3151. It still lacks the deprecation mechanism mentioned in "step 1" of the PEP.

The implementation is at http://hg.python.org/features/pep-3151/ in the branch named "pep-3151".
msg140315 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-07-13 22:39
(I'm sorry for the filenames. I'm using the "create patch" feature in the hope that a code review is possible through the Rietveld integration :-))
msg140743 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-07-20 14:47
The patch looks good.  I can’t comment on details of the C code, but I’ve seen the changed Python code and the tests and I like this.
msg142210 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-08-16 18:08
Updated patch with latest changes from the PEP. The implementation is now complete.
msg142217 - (view) Author: Remi Pointel (rpointel) Date: 2011-08-16 20:20
Hi,
I have tested on OpenBSD -current, and it seems to work fine:

$ ./python -E -bb Lib/test/test_mmap.py          
test_access_parameter (__main__.MmapTests) ... ok
test_anonymous (__main__.MmapTests) ... ok
test_bad_file_desc (__main__.MmapTests) ... ok
test_basic (__main__.MmapTests) ... ok
test_context_manager (__main__.MmapTests) ... ok
test_context_manager_exception (__main__.MmapTests) ... ok
test_double_close (__main__.MmapTests) ... ok
test_entire_file (__main__.MmapTests) ... ok
test_error (__main__.MmapTests) ... ok
test_extended_getslice (__main__.MmapTests) ... ok
test_extended_set_del_slice (__main__.MmapTests) ... ok
test_find_end (__main__.MmapTests) ... ok
test_io_methods (__main__.MmapTests) ... ok
test_length_0_large_offset (__main__.MmapTests) ... ok
test_length_0_offset (__main__.MmapTests) ... ok
test_move (__main__.MmapTests) ... ok
test_non_ascii_byte (__main__.MmapTests) ... ok
test_offset (__main__.MmapTests) ... ok
test_prot_readonly (__main__.MmapTests) ... ok
test_read_all (__main__.MmapTests) ... ok
test_read_invalid_arg (__main__.MmapTests) ... ok
test_rfind (__main__.MmapTests) ... ok
test_subclass (__main__.MmapTests) ... ok
test_tougher_find (__main__.MmapTests) ... ok
test_around_2GB (__main__.LargeMmapTests) ... ok
test_around_4GB (__main__.LargeMmapTests) ... ok
test_large_filesize (__main__.LargeMmapTests) ... ok
test_large_offset (__main__.LargeMmapTests) ... ok

----------------------------------------------------------------------
Ran 28 tests in 0.141s

OK
$ ./python -E -bb Lib/test/test_exceptions.py
...........................
----------------------------------------------------------------------
Ran 27 tests in 0.026s

OK
$ ./python -E -bb Lib/test/test_pep3151.py
test_blockingioerror (__main__.AttributesTest) ... ok
test_errno_translation (__main__.AttributesTest) ... skipped 'Windows-specific test'
test_posix_error (__main__.AttributesTest) ... ok
test_windows_error (__main__.AttributesTest) ... ok
test_builtin_errors (__main__.HierarchyTest) ... ok
test_errno_mapping (__main__.HierarchyTest) ... ok
test_ioerror_subclass_mapping (__main__.HierarchyTest) ... ok
test_select_error (__main__.HierarchyTest) ... ok
test_socket_errors (__main__.HierarchyTest) ... ok

----------------------------------------------------------------------
Ran 9 tests in 0.001s

OK (skipped=1)


Don't hesitate to indicate me if you need more tests, or if I did something wrong.

Cheers,
Remi.
msg142556 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-08-20 19:23
New patch incorporating Ezio's comments and synchronized with latest default.
msg143225 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-08-30 17:00
Here is a new patch implementing the latest PEP changes.
msg143758 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2011-09-09 10:01
I still need to understand the full impact of the PEP, but it seems
very sound. I've left two small comments on Rietveld, and there's
an additional question:


Should the input of OSError be checked?

>>> OSError("not an errno", "Bad file descriptor")
OSError('not an errno', 'Bad file descriptor')
>>> o = OSError("not an errno", "Bad file descriptor")
>>> raise o
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno not an errno] Bad file descriptor
>>>
msg144829 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-10-03 18:30
> Should the input of OSError be checked?

It could, but pre-PEP it is not, so I assumed it's better to minimize compatibility-breaking changes.
msg144830 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-10-03 18:32
Patch update against latest default. There shouldn't be anything interesting to see.
msg145380 - (view) Author: Roundup Robot (python-dev) Date: 2011-10-12 01:00
New changeset 41a1de81ef2b by Antoine Pitrou in branch 'default':
PEP 3151 / issue #12555: reworking the OS and IO exception hierarchy.
http://hg.python.org/cpython/rev/41a1de81ef2b
msg145381 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-10-12 01:05
Thanks everyone for the reviews!
msg148829 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * Date: 2011-12-04 03:33
Is the following change in behavior caused by the fix for this issue?

$ python3.2 -c $'class A(IOError):\n  def __init__(self, arg): pass\nA(arg=1)'
$ python3.3 -c $'class A(IOError):\n  def __init__(self, arg): pass\nA(arg=1)'
Traceback (most recent call last):
  File "<string>", line 3, in <module>
TypeError: A does not take keyword arguments
msg148832 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-12-04 04:10
Indeed, this seems to be the most likely culprit for the current buildbot failures in the new urllib2 tests:

http://www.python.org/dev/buildbot/all/builders/AMD64%20Gentoo%20Wide%203.x/builds/2868/steps/test/logs/stdio
msg148836 - (view) Author: Roundup Robot (python-dev) Date: 2011-12-04 04:20
New changeset a3ddee916808 by Jason R. Coombs in branch 'default':
Pass positional arguments - HTTPError is not accepting keyword arguments. Reference #13211 and #12555.
http://hg.python.org/cpython/rev/a3ddee916808
msg148844 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-04 08:59
> Is the following change in behavior caused by the fix for this issue?
> 
> $ python3.2 -c $'class A(IOError):\n  def __init__(self, arg): pass\nA(arg=1)'
> $ python3.3 -c $'class A(IOError):\n  def __init__(self, arg): pass\nA(arg=1)'
> Traceback (most recent call last):
>   File "<string>", line 3, in <module>
> TypeError: A does not take keyword arguments

It must be because IOError now has a significant __new__ method.
I could change it to accept arbitrary arguments but I'm not sure that's
the right solution.
Another approach would be:
- if IOError is instantiated, initialize stuff in IOError.__new__
- otherwise, initialize stuff in IOError.__init__

What do you think?
msg148845 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-04 09:19
> > Is the following change in behavior caused by the fix for this issue?
> > 
> > $ python3.2 -c $'class A(IOError):\n  def __init__(self, arg): pass\nA(arg=1)'
> > $ python3.3 -c $'class A(IOError):\n  def __init__(self, arg): pass\nA(arg=1)'
> > Traceback (most recent call last):
> >   File "<string>", line 3, in <module>
> > TypeError: A does not take keyword arguments
> 
> It must be because IOError now has a significant __new__ method.
> I could change it to accept arbitrary arguments but I'm not sure that's
> the right solution.
> Another approach would be:
> - if IOError is instantiated, initialize stuff in IOError.__new__
> - otherwise, initialize stuff in IOError.__init__

To make things clearer, IOError.__new__ would detect if a subclass is
asked for, and then defer initialization until __init__ is called so
that argument checking is done in __init__.
msg148849 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-12-04 09:30
There's a fairly sophisticated tapdance in object.__new__ that deals with this problem at that level.

See: http://hg.python.org/cpython/file/default/Objects/typeobject.c#l2869

The new IOError may require something similarly sophisticated to cope with subclasses that only override __init__.
msg149520 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-15 09:47
Here is a patch. Can someone take a look?
msg149549 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-15 13:06
Updated patch. I've renamed oserror_post_init to oserror_init. Also addressed Nick's other comments.
msg149554 - (view) Author: Roundup Robot (python-dev) Date: 2011-12-15 13:33
New changeset d22c99e77768 by Antoine Pitrou in branch 'default':
Fix OSError.__init__ and OSError.__new__ so that each of them can be
http://hg.python.org/cpython/rev/d22c99e77768
msg149555 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-15 13:34
Fixed now.
History
Date User Action Args
2011-12-15 13:34:41pitrousetstatus: open -> closed
resolution: fixed
messages: + msg149555

stage: needs patch -> committed/rejected
2011-12-15 13:33:09python-devsetmessages: + msg149554
2011-12-15 13:06:37pitrousetfiles: + oserror_new2.patch

messages: + msg149549
2011-12-15 09:47:03pitrousetfiles: + oserror_new.patch

messages: + msg149520
2011-12-06 23:21:19ncoghlansetpriority: normal -> release blocker
2011-12-04 09:30:03ncoghlansetmessages: + msg148849
2011-12-04 09:19:17pitrousetmessages: + msg148845
2011-12-04 08:59:11pitrousetmessages: + msg148844
2011-12-04 04:20:41python-devsetmessages: + msg148836
2011-12-04 04:10:21ncoghlansetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg148832

stage: committed/rejected -> needs patch
2011-12-04 03:33:28Arfreversetnosy: + Arfrever
messages: + msg148829
2011-10-12 01:05:26pitrousetstatus: open -> closed
resolution: fixed
messages: + msg145381

stage: patch review -> committed/rejected
2011-10-12 01:00:50python-devsetnosy: + python-dev
messages: + msg145380
2011-10-03 18:32:48pitrousetmessages: + msg144830
2011-10-03 18:31:56pitrousetfiles: + pep3151-6.diff
2011-10-03 18:30:50pitrousetmessages: + msg144829
2011-09-09 10:01:56skrahsetmessages: + msg143758
2011-08-30 17:00:13pitrousetmessages: + msg143225
2011-08-30 16:57:51pitrousetfiles: + pep3151-5.diff
2011-08-20 19:23:58pitrousetmessages: + msg142556
2011-08-20 19:22:40pitrousetfiles: + pep3151-4.diff
2011-08-16 20:20:32rpointelsetnosy: + rpointel
messages: + msg142217
2011-08-16 18:08:55pitrousetmessages: + msg142210
2011-08-16 18:06:45pitrousetfiles: + pep3151-3.diff
2011-07-21 22:33:32pitrousetfiles: - 40ae82fafaed.diff
2011-07-21 22:31:54pitrousetfiles: + pep3151-2.diff
2011-07-21 18:57:28Trundlesetnosy: + Trundle
2011-07-20 14:47:08eric.araujosetnosy: + eric.araujo
messages: + msg140743
2011-07-20 14:45:37pitrousetcomponents: + Library (Lib)
stage: patch review
2011-07-20 14:38:04eric.araujosetfiles: + 40ae82fafaed.diff
2011-07-15 07:47:17djcsetnosy: + djc
2011-07-14 22:30:31skrahsetnosy: + skrah
2011-07-14 19:40:04petri.lehtinensetnosy: + petri.lehtinen
2011-07-13 23:44:07pitrousetfiles: - d305942126b6.diff
2011-07-13 23:40:36pitrousetfiles: + pep3151-1.diff
2011-07-13 22:39:01pitrousetmessages: + msg140315
2011-07-13 22:35:33pitrousetfiles: + d305942126b6.diff
2011-07-13 22:32:58nadeem.vawdasetnosy: + nadeem.vawda
2011-07-13 22:28:51pitrousetfiles: - cac46b853098.diff
2011-07-13 22:20:47pitrousetfiles: + cac46b853098.diff
keywords: + patch
2011-07-13 22:19:46pitrousethgrepos: + hgrepo41
2011-07-13 22:19:31pitroucreate