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

eliben at gmail.com eliben at gmail.com
Sat May 11 05:23:58 CEST 2013


Please use the code-review tool to either mark comments "Done" or
explain/disagree/discuss, before uploading the next patch.


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

http://bugs.python.org/review/17947/diff/8113/Lib/enum.py#newcode1
Lib/enum.py:1: """\
Again, *please* use PEP 257 convention. Even if you don't find the
multi-line convention pretty (I personally don't), it's far more
important for the code base to be consistently formatted than any
particular style.

One of the best things of Python as an open-source project is the
relative consistency of its source code due in terms of style.

http://bugs.python.org/review/17947/diff/8113/Lib/enum.py#newcode13
Lib/enum.py:13: """
PEP 257 everywhere...

"""First line of multi-line docstring.
Second line.
Third line. getting boring. Before the closing triple-quote we have an
empty line

"""

http://bugs.python.org/review/17947/diff/8113/Lib/enum.py#newcode101
Lib/enum.py:101: """\
"""Metaclass for Enum"""

http://bugs.python.org/review/17947/diff/8113/Lib/enum.py#newcode115
Lib/enum.py:115: __new__, save_new, use_args =
metacls._find_new(classdict, obj_type, first_enum)
80-column violation

http://bugs.python.org/review/17947/diff/8113/Lib/enum.py#newcode174
Lib/enum.py:174: def __call__(cls, value, names=None, *, module=None,
type=None):
Explain the semantics of __call__ in detail in a docstring

http://bugs.python.org/review/17947/diff/8113/Lib/enum.py#newcode214
Lib/enum.py:214: return cls._enum_map.copy()
I still don't like this copy(). Can you say what it is for? If someone
wants to screw an Enum up he can access _enum_map directly - Python
doesn't excel at data hiding. OTOH, copy needlessly makes innocent code
(99.99% of the code) slower.

http://bugs.python.org/review/17947/diff/8113/Lib/enum.py#newcode217
Lib/enum.py:217: """Return the enum item matching `name`"""
s/item/member/

Use members consistently everywhere

http://bugs.python.org/review/17947/diff/8113/Lib/enum.py#newcode218
Lib/enum.py:218: if name[:2] == name[-2:] == '__':
You have this more than once in the code. Have a helper method
_is_dundername or something

http://bugs.python.org/review/17947/diff/8113/Lib/enum.py#newcode275
Lib/enum.py:275: # now find the correct __new__, checking to see of one
was defined
Here and everywhere - use docstrings to provide a method documentation
note. Explain what each argument needs.

Inside the method you can use comments freely

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


More information about the docs mailing list