I am sending this review as the BDFOP for PEP 3151. I've read the PEP and
reviewed the python-dev discussion via Gmane. I have not reviewed the hg
branch where Antoine has implemented it.
I'm not quite ready to pronounce, but I do have some questions and comments.
First off, thanks to Antoine for taking this issue on, and for his well
written and well reasoned PEP. There's definitely a problem here and I think
Python will be better off for having addressed it. I, for one, will be very
happy when I can eliminate the majority of `import errno`s from my code. ;)
One guiding principle for me is that we should keep the abstraction as thin as
possible. In particular, I'm concerned about mapping multiple errnos into a
single Error. For example both EPIPE and ESHUTDOWN mapping to BrokePipeError,
or EACESS or EPERM to PermissionError. I think we should resist this, so that
one errno maps to exactly one Error. Where grouping is desired, Python
already has mechanisms to deal with that, e.g. superclasses and multiple
inheritance. Therefore, I think it would be better to have
+ AccessError (EACCES)
+ PermissionError (EPERM)
Yes, it makes the hierarchy deeper, and means you have to come up with a few
more names, but I think it will also make it easier for the programmer to use
and debug. Also, some of the artificial hierarchy introduced in the PEP may
not be necessary (e.g. the faux superclass FileSystemPermissionError above).
This might lead to the elimination of FileSystemError as some have suggested
(I too question its utility).
Similarly, I think it would be helpful to have the errno name (e.g. ENOENT) in
the error message string. That way, it won't get in the way for most code,
but would be usefully printed out for uncaught exceptions.
A second guiding principle should be that careful code that works in Python
3.2 must continue to work in Python 3.3 once PEP 3151 is accepted, but also
for Python 2 code ported straight to Python 3.3. Given the PEP's emphasis on
"useful compatibility", I think this will be the case. Do be prepared for
complaints about compatibility for careless code though - there's a ton of
that out in the wild, and people will always complain with their "working"
code breaks due to an upgrade. Be *very* explicit about this in the release
notes and NEWS file, and put your asbestos underoos on. On the plus side,
there's not so much Python 3 code to break :). Also, do clearly explain any
required migration strategy for existing code, probably in this PEP.
Have you considered the impact of this PEP on other Python implementations?
My hazy memory of Jython tells me that errnos don't really leak into Java and
thus Jython much, but what about PyPy and IronPython? E.g. step 1's
deprecation strategy seems pretty CPython-centric.
As for step 1 (coalescing the errors). This makes sense and I'm generally
agreeable, but I'm wondering whether it's best to re-use IOError for this
rather than introduce a new exception. Not that I can think of a good name
for that. I'm just not totally convinced that existing code when upgrading to
Python 3.3 won't introduce silent failures. If an existing error is to be
re-used for this, I'm torn on whether IOError or OSError is a better choice.
Popularity aside, OSError *feels* more right.
What is the impact of the PEP on tools such as 2to3 and 3to2?
Just to be clear, am I right that (on POSIX systems at least) IOError and its
subclasses will always have an errno attribute still? And that anything
raising an exception (e.g. via PyErr_SetFromErrno) other than the new ones
will raise IOError?
I also think that rather than transforming exception when raised from Python,
i.e. via __new__ hackery, perhaps it should be a ValueError in its own right
to raise IOError with an error represented by one of the subclasses. Chained
exceptions would mean that the original exception needn't get lost.
I surveyed some of my own code and observed (as others have) that EISDIR and
ENOTDIR are pretty rare. I found more examples of ECHILD and ESRCH than the
former two. How'd you like to add those two to make your BDFOP happy? :)
What follows are some crazier ideas. I'm just throwing them out there, not
necessarily suggesting they should go into the PEP.
The new syntax (e.g. if clause on except) is certainly appealing at first
glance, and might be of more general use for Python, but I agree with the
problems as stated in the PEP. However, there might be a few things that
*can* be done to make even the uncommon cases easier. E.g.
What if all the errno symbolic names were mapped as attributes on IOError?
The only advantage of that would be to eliminate the need to import errno, or
for the ugly `e.errno == errno.ENOENT` stuff. That would then be rewritten as
`e.errno == IOError.ENOENT`. A mild savings to be sure, but still.
How dumb/useless/unworkable would it be to add an __future__ to switch from
the old hierarchy to the new one? Probably pretty. ;)
What about an api that applications/libraries could use to add additional
exceptions based on other errnos they cared about? This could be consulted in
PyErr_SetFromErrno() and raised instead of IOError. Okay, yeah, that's
probably pretty dumb too.
Anyway, that's all I have. I certainly feel like this PEP is pretty close to
being accepted. Good work!