classification
Title: Revise PEP 8 recommendation for class names
Type: Stage: patch review
Components: Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: barry Nosy List: barry, ethan.furman, ncoghlan, paul.moore, pitrou, r.david.murray
Priority: normal Keywords: patch

Created on 2013-10-21 14:01 by barry, last changed 2013-11-01 16:57 by barry. This issue is now closed.

Files
File name Uploaded Description Edit
issue19331.stoneleaf.01.patch ethan.furman, 2013-10-22 11:35
issue19331.stoneleaf.02.patch ethan.furman, 2013-10-23 21:05
issue19331-barry.diff barry, 2013-10-24 13:28
issue19331.stoneleaf.03.patch ethan.furman, 2013-11-01 00:56
Messages (22)
msg200783 - (view) Author: Barry A. Warsaw (barry) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-10-21 20:41
That said, I quite like Paul's suggestions.
msg200845 - (view) Author: Barry A. Warsaw (barry) * (Python committer) 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) * (Python committer) Date: 2013-10-22 11:34
Attached patch combines Paul's, Antoine's, and Nick's suggestions/observations.
msg200942 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) 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) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2013-10-24 13:54
-1 on Barry's wording and +1 on Nick's wording in msg201119.
msg201136 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) 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) * (Python committer) 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: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-11-01 13:24
+1, works for me :)
msg201914 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2013-11-01 16:57
Thanks Ethan, your latest patch is wonderful.  Applied!
msg201915 - (view) Author: Roundup Robot (python-dev) (Python triager) 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
History
Date User Action Args
2013-11-01 16:57:15barrysetstatus: open -> closed
nosy: - python-dev
resolution: fixed
2013-11-01 16:57:05python-devsetnosy: + python-dev
messages: + msg201915
2013-11-01 16:57:05barrysetmessages: + msg201914
2013-11-01 13:24:18ncoghlansetmessages: + msg201896
2013-11-01 00:56:55ethan.furmansetfiles: + issue19331.stoneleaf.03.patch

messages: + msg201864
2013-10-24 13:55:00ncoghlansetmessages: + msg201136
2013-10-24 13:54:08pitrousetmessages: + msg201135
2013-10-24 13:53:17pitrousetmessages: + msg201134
2013-10-24 13:28:39barrysetfiles: + issue19331-barry.diff

messages: + msg201127
2013-10-24 12:45:13paul.mooresetmessages: + msg201121
2013-10-24 12:33:42ncoghlansetmessages: + msg201119
2013-10-24 12:08:06pitrousetmessages: + msg201117
2013-10-24 11:01:48ncoghlansetmessages: + msg201112
2013-10-24 09:19:21pitrousetmessages: + msg201105
2013-10-23 21:05:45ethan.furmansetfiles: + issue19331.stoneleaf.02.patch
2013-10-22 13:33:10paul.mooresetmessages: + msg200946
2013-10-22 13:16:43ncoghlansetmessages: + msg200942
2013-10-22 11:35:25ethan.furmansetstage: patch review
2013-10-22 11:35:09ethan.furmansetfiles: + issue19331.stoneleaf.01.patch
keywords: + patch
2013-10-22 11:34:56ethan.furmansetmessages: + msg200918
2013-10-21 20:51:51barrysetmessages: + msg200845
2013-10-21 20:41:05ncoghlansetmessages: + msg200841
2013-10-21 20:39:53ncoghlansetmessages: + msg200840
2013-10-21 17:56:00pitrousetnosy: + ncoghlan
2013-10-21 17:55:46pitrousetnosy: + pitrou
messages: + msg200817
2013-10-21 17:40:44r.david.murraysetnosy: + r.david.murray
2013-10-21 16:59:43ethan.furmansetnosy: + ethan.furman
2013-10-21 14:18:30paul.mooresetnosy: + paul.moore
messages: + msg200786
2013-10-21 14:02:39barrysetassignee: barry
2013-10-21 14:01:48barrycreate