| msg200783 - (view) |
Author: Barry A. Warsaw (barry) *  |
Date: 2013-10-21 14:01 |
| PEP 8 says: """ Class Names Almost without exception, class names use the CapWords convention. Classes for internal use have a leading underscore in addition. """ yet there are some notable exceptions in practice, such as classes designed to be context managers (used with the `with` statement). This message indicates Nick's implementation choice of a wrapper function in part to avoid naming a class with all lower cases (which look better with `with`, but break the PEP 8 convention). https://mail.python.org/pipermail/python-dev/2013-October/129791.html The PEP 8 language should be revised, but striking the right balance might be a little tricky. |
|
|
| msg200786 - (view) |
Author: Paul Moore (paul.moore) * |
Date: 2013-10-21 14:18 |
| How about simply adding a further sentence, something like: "Where a class is being used conceptually as a callable (for example, context managers), the naming convention for callables (lower_case_with_underscores) should be followed." Maybe also add a section at the start of "Naming Conventions": """ Overriding Principle Whenever names are visible to the user, as public parts of the API, the naming convention should reflect usage rather than implementation. An internal change (for example switching from a class to a factory function) should never affect the names used in the public API. """ |
|
|
| msg200817 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-10-21 17:55 |
| s/should be followed/may be followed/ As an other point of reference, for a long time the synchronization classes in the threading module were actually mediated by function wrappers, e.g.: def Lock(*args, **kwargs): return _Lock(*args, **kwargs) (this was to "discourage subclassing", IIRC) |
|
|
| msg200840 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2013-10-21 20:39 |
| Lots of builtins, collections and itertools use lowercase names as well, as do some older stdlib types (array, mmap, socket). The only clear dividing lines I can really discern are that ABCs *must* start with a capital, as should classes paired with a separate factory function. |
|
|
| msg200841 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2013-10-21 20:41 |
| That said, I quite like Paul's suggestions. |
|
|
| msg200845 - (view) |
Author: Barry A. Warsaw (barry) * |
Date: 2013-10-21 20:51 |
| On Oct 21, 2013, at 08:41 PM, Nick Coghlan wrote: >That said, I quite like Paul's suggestions. As do I. |
|
|
| msg200918 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2013-10-22 11:34 |
| Attached patch combines Paul's, Antoine's, and Nick's suggestions/observations. |
|
|
| msg200942 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2013-10-22 13:16 |
| Hmm, review link doesn't appear to be available, so just commenting here. For the "overriding principle" part, you can't switch from a class to a factory function (as that's a backwards incompatible change due to breaking subclassing and isinstance and issubclass checks). The relevant example is going the other way (from a function to relying on a class __new__ or __init__ method instead). For the addition to the class names section, you can't really jam Paul's wording and mine together like that, as it makes the "this rule" reference ambiguous as to whether it's referring to the previous sentence (intended) or the part before the semi-colon (not intended). "ABC classes" also doesn't work, since that ends with a repeated "class". Perhaps: ================= Class names should normally use the CapWords convention, with classes intended solely for internal use starting with a leading underscore. This guideline should always be followed for abstract base classes, any other classes where inheritance is encouraged, and classes that are paired with a separate factory function. In cases where inheritance is not encouraged and it is judged to improve readability at the point of use, the naming convention for callables (lower_case_with_underscores) may be used instead. This is an indication that the type is intended primarily for use "as is", rather than through inheritance (although subclassing is still permitted). Note that there is a separate convention for builtin names: most builtin names are single words (or two words run together), with the CapWords convention used only for exception names and builtin constants. ================= |
|
|
| msg200946 - (view) |
Author: Paul Moore (paul.moore) * |
Date: 2013-10-22 13:33 |
| +1 on switching the wording in the overriding principle section to say "factory function to class". Although the fact that I made that mistake reflects my point that if I'm thinking of class->wrapper as an "internal change" then I'm not anticipating or supporting subclassing. (That's my excuse and I'm sticking to it :-)) Nick's revised wording for the class names section took me a couple of reads to follow - it feels a little too complex as a result of trying to cover everything. But I couldn't come up with anything substantially better, so call me +0 on it. |
|
|
| msg201105 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-10-24 09:19 |
| > In cases where inheritance is not encouraged and it is judged to improve > readability at the point of use, the naming convention for callables > (lower_case_with_underscores) may be used instead. This is an indication > that the type is intended primarily for use "as is", rather than through > inheritance (although subclassing is still permitted). I don't think this wording is appropriate. As soon as the "thing" is documented as a *type* (i.e. something you call to get instances that have a specific interface - methods, etc.), then IMO it should follow the naming scheme for classes. Only when the "thing" is not documented as a type but as a convenience callable (for example a context manager) is it reasonable to follow the naming scheme for functions. In other words, this has nothing to do with subclassing. |
|
|
| msg201112 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2013-10-24 11:01 |
| collections.defaultdict, collections.deque, array.array and many of the builtins suggest the rule isn't that simple (although other examples like Decimal, Fraction, OrderedDict, Counter and ChainMap suggest my guideline is also wrong). The main thing I want is to be able to provide and document attributes on contextlib.suppress and contextlib.redirect_stdio without violating PEP 8. |
|
|
| msg201117 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-10-24 12:08 |
| > collections.defaultdict, collections.deque, array.array and many of > the builtins suggest the rule isn't that simple (although other > examples like Decimal, Fraction, OrderedDict, Counter and ChainMap > suggest my guideline is also wrong). AFAIK, they are simply old enough to predate the rule or its wide-spread acceptance. There *is* an exception for builtins (bytearray and memoryview are recent enough), but most recent types use CamelCase. The bottom line, though, is that it's unrelated to subclassing, therefore that particular piece of explanation shouldn't land in PEP 8 :-) |
|
|
| msg201119 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2013-10-24 12:33 |
| Documenting it as a callable is a pretty strong hint that it shouldn't be subclassed, but I agree my wording above is wrong. Next attempt (leaving out the mention of leading underscores, since that is covered elsewhere): ================= Class Names ----------- Class names should normally use the CapWords convention. The naming convention for functions may be used instead in cases where the interface is documented and used primarily as a callable. Note that there is a separate convention for builtin names: most builtin names are single words (or two words run together), with the CapWords convention used only for exception names and builtin constants. ================= I initially had the following appended to the second paragraph: "... , and instances expose no public mutable attributes or instance methods (but may provide read-only data attributes, alternative constructors and appropriate special methods)". The longer phrasing was designed to explicitly cover itertools.chain.from_iterable, contextlib.redirect_stdio.target and contextlib.suppress.exceptions, but I don't think that level of detail is necessary. |
|
|
| msg201121 - (view) |
Author: Paul Moore (paul.moore) * |
Date: 2013-10-24 12:45 |
| I'm in favour of keeping it simple - "use your common sense" should be a general guideline here (even though I know people's tastes differ, and that can be a source of endless debate :-)) |
|
|
| msg201127 - (view) |
Author: Barry A. Warsaw (barry) * |
Date: 2013-10-24 13:28 |
| I always prefer to keep PEP 8 guidelines as succinct as possible. I've looked over Ethan's patch and the other changes suggested in comments and come up with what I think is a simple patch to improve the guidelines. I don't think we need to go into a lot of detail here, since the primary goal of the change is to relax the constraints to allow the changes in contextlib to not violate PEP 8. |
|
|
| msg201134 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-10-24 13:53 |
| > I always prefer to keep PEP 8 guidelines as succinct as possible. > I've looked over Ethan's patch and the other changes suggested in > comments and come up with what I think is a simple patch to improve > the guidelines. Have you read what I wrote? Inheritance should be left out of it. |
|
|
| msg201135 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2013-10-24 13:54 |
| -1 on Barry's wording and +1 on Nick's wording in . |
|
|
| msg201136 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2013-10-24 13:55 |
| Barry, Antoine convinced me that my wording from earlier was both inaccurate and overly complicated. I'm happy with the simplified version just above Paul's last comment, though, so combining that with the earlier part of your patch looks promising to me. |
|
|
| msg201864 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2013-11-01 00:56 |
| How's this? I liked Barry's simpler Overriding Principle combine with Nick's simpler Class Names. |
|
|
| msg201896 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2013-11-01 13:24 |
| +1, works for me :) |
|
|
| msg201914 - (view) |
Author: Barry A. Warsaw (barry) * |
Date: 2013-11-01 16:57 |
| Thanks Ethan, your latest patch is wonderful. Applied! |
|
|
| msg201915 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-11-01 16:57 |
| New changeset 1a40d4eaa00b by Barry Warsaw in branch 'default': Ethan Furman's latest patch for Issue 19331. http://hg.python.org/peps/rev/1a40d4eaa00b |
|
|