[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