[docs] Code, test, and doc review for PEP-0435 Enum (issue 17947)

zachary.ware at gmail.com zachary.ware at gmail.com
Fri May 17 15:37:11 CEST 2013


Here's the latest nitpicks from me :)


http://bugs.python.org/review/17947/diff/8131/Lib/enum.py
File Lib/enum.py (right):

http://bugs.python.org/review/17947/diff/8131/Lib/enum.py#newcode92
Lib/enum.py:92: metacls._find_new(classdict, obj_type, first_enum)
On 2013/05/13 20:32:37, stoneleaf wrote:
> On 2013/05/13 19:06:12, zach.ware wrote:
> > PEP 8 recommends avoiding backslashes for line continuation; it
would be
> better
> > to split the line in the _find_new call, I think.
> > 
> > """
> >         __new__, save_new, use_args = metacls._findnew(classdict,
> >                                                        obj_type,
> >                                                        first_enum)
> > """
> 
> How about:
> 
> """
>         __new__, save_new, use_args = (
>                 metacls._find_new(classdict, obj_type, first_enum)
>                 )
> """
> 
> ?

That's not my preference, but I suppose it's better than the backslash.

http://bugs.python.org/review/17947/diff/8161/Lib/enum.py
File Lib/enum.py (right):

http://bugs.python.org/review/17947/diff/8161/Lib/enum.py#newcode7
Lib/enum.py:7: import weakref
As pointed out by Donald Stufft on the issue, weakref is unused.

(Just to have a comment on it :))

http://bugs.python.org/review/17947/diff/8161/Lib/enum.py#newcode37
Lib/enum.py:37: def dunder(name):
Might be better named "is_dunder" since you're asking if it is, not
making it so.

http://bugs.python.org/review/17947/diff/8161/Lib/enum.py#newcode52
Lib/enum.py:52: raise TypeError('%r cannot be pickled' % self)
These two functions should be made private unless there's good reason
for them to be public.  Hard to change later if they're public (even if
they're left out of __all__).

http://bugs.python.org/review/17947/diff/8161/Lib/enum.py#newcode123
Lib/enum.py:123: "'_find_new' cannot be used for members")
I wonder if it would be better to explicitly disallow any _name?  Or, I
think I read somewhere (possibly in the enum discussions, though I don't
remember for sure :)) that the implementer of namedtuple later wished
they had used names_ instead of _names and reserved that whole
namespace_; perhaps that would be a good idea here.

http://bugs.python.org/review/17947/diff/8161/Lib/test/test_enum.py
File Lib/test/test_enum.py (right):

http://bugs.python.org/review/17947/diff/8161/Lib/test/test_enum.py#newcode2
Lib/test/test_enum.py:2: import sys
Turns out sys isn't used here, either.

http://bugs.python.org/review/17947/


More information about the docs mailing list