Further PEP 8 compliance issues in threading and multiprocessing
I've been taking a close look at the API for multiprocessing and threading, and have discovered a somewhat strange pattern that occurs multiple times in both interfaces: factory functions with names that start with a capital letter so they look like they're actually a class. At first I thought it was a quirk of the multiprocessing implementation, but then I discovered that the threading module also does it for all of the synchronisation objects as well as threading.Timer, with examples like: def Event(*args, **kwds): return _Event(*args, **kwds) Is this just intended to discourage subclassing? If so, why give the misleading impression that these things can be subclassed by naming them as if they were classes? How should this be handled when it comes to the addition of PEP 8 compliant aliases? Add aliases for the factory functions that start with a lower case letter, but otherwise leave the structure of the modules unchanged? Or should the trivial functions like the one above be dropped and made direct aliases for the classes they are currently wrapping? Option 1 has the virtue of being perfectly backwards compatible in the threading case, while option 2 is a little riskier, so I'm inclined to go with option 1 (keeping the factory functions, but giving them non-class like names in the case of multiprocessing, or aliases in the case of threading). Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia --------------------------------------------------------------- http://www.boredomandlaziness.org
Nick Coghlan <ncoghlan <at> gmail.com> writes:
Is this just intended to discourage subclassing? If so, why give the misleading impression that these things can be subclassed by naming them as if they were classes?
How should this be handled when it comes to the addition of PEP 8 compliant aliases?
I don't see a problem for trivial functional wrappers to classes to be capitalized like classes. So I'd suggest option 3: leave it as-is. Otherwise option 2 (replace the wrappers with the actual classes) has my preference.
On Mon, Sep 1, 2008 at 9:36 AM, Antoine Pitrou <solipsis@pitrou.net> wrote:
Nick Coghlan <ncoghlan <at> gmail.com> writes:
Is this just intended to discourage subclassing? If so, why give the misleading impression that these things can be subclassed by naming them as if they were classes?
How should this be handled when it comes to the addition of PEP 8 compliant aliases?
I don't see a problem for trivial functional wrappers to classes to be capitalized like classes. So I'd suggest option 3: leave it as-is. Otherwise option 2 (replace the wrappers with the actual classes) has my preference.
Yes, I believe that pretending that functions are classes is a fairly common idiom in the stdlib and out, so I see no problem leaving them alone. We haven't had any complaints about the threading Event function yet either. :) -- Cheers, Benjamin Peterson "There's no place like 127.0.0.1."
On Mon, 1 Sep 2008 09:42:06 -0500, Benjamin Peterson <musiccomposition@gmail.com> wrote:
On Mon, Sep 1, 2008 at 9:36 AM, Antoine Pitrou <solipsis@pitrou.net> wrote:
Nick Coghlan <ncoghlan <at> gmail.com> writes:
Is this just intended to discourage subclassing? If so, why give the misleading impression that these things can be subclassed by naming them as if they were classes?
How should this be handled when it comes to the addition of PEP 8 compliant aliases?
I don't see a problem for trivial functional wrappers to classes to be capitalized like classes. So I'd suggest option 3: leave it as-is. Otherwise option 2 (replace the wrappers with the actual classes) has my preference.
Yes, I believe that pretending that functions are classes is a fairly common idiom in the stdlib and out, so I see no problem leaving them alone. We haven't had any complaints about the threading Event function yet either. :)
Here's a complaint. It's surprising that you can't use Event et al with isinstance. This is something I'm sure a lot of people run into (I did, many years ago) when they start to use these APIs. Once you do figure out why it doesn't work, it's not clear how to do what you want, since _Event is private. Jean-Paul
Jean-Paul Calderone <exarkun <at> divmod.com> writes:
Here's a complaint. It's surprising that you can't use Event et al with isinstance. This is something I'm sure a lot of people run into (I did, many years ago) when they start to use these APIs. Once you do figure out why it doesn't work, it's not clear how to do what you want, since _Event is private.
event_class = Event().__class__ ? Not pretty I know :-)
On Mon, Sep 1, 2008 at 10:00 AM, Jean-Paul Calderone <exarkun@divmod.com> wrote:
On Mon, 1 Sep 2008 09:42:06 -0500, Benjamin Peterson
Yes, I believe that pretending that functions are classes is a fairly common idiom in the stdlib and out, so I see no problem leaving them alone. We haven't had any complaints about the threading Event function yet either. :)
Here's a complaint. It's surprising that you can't use Event et al with isinstance. This is something I'm sure a lot of people run into (I did, many years ago) when they start to use these APIs. Once you do figure out why it doesn't work, it's not clear how to do what you want, since _Event is private.
Does anybody ever complain about not being able to use isinstance on twisted.application.Application? (At least it's documented as a function there.)
Jean-Paul _______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/musiccomposition%40gmail.c...
-- Cheers, Benjamin Peterson "There's no place like 127.0.0.1."
Fredrik Lundh wrote:
Benjamin Peterson wrote:
Does anybody ever complain about not being able to use isinstance on twisted.application.Application? (At least it's documented as a function there.)
the threading "non-classes" are documented to be factory functions on the module page.
Given the proximity of RC1, Antoine's option 3 (leaving the capitalised factory functions in both multiprocessing and threading APIs) is actually sounding pretty appealing to me at the moment. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia --------------------------------------------------------------- http://www.boredomandlaziness.org
Nick Coghlan <ncoghlan <at> gmail.com> writes:
Given the proximity of RC1, Antoine's option 3 (leaving the capitalised factory functions in both multiprocessing and threading APIs) is actually sounding pretty appealing to me at the moment.
I was actually suggesting this course for the threading module, whose API has existed for a lot of time now, but I think it would be better to clean up multiprocessing before its first stable relase. But the issue of time and manpower starts being a bit critical :)
Antoine Pitrou wrote:
Nick Coghlan <ncoghlan <at> gmail.com> writes:
Given the proximity of RC1, Antoine's option 3 (leaving the capitalised factory functions in both multiprocessing and threading APIs) is actually sounding pretty appealing to me at the moment.
I was actually suggesting this course for the threading module, whose API has existed for a lot of time now, but I think it would be better to clean up multiprocessing before its first stable relase. But the issue of time and manpower starts being a bit critical :)
Unfortunately, that's where the whole "close to a drop-in replacement for threading" concept brings additions to the threading module API back into play. If I'd realised this a bit sooner I probably would have been pushing for it to be dealt with for 2.6/3.0, but given that it's the kind of change that we can easily do through the normal API deprecation process, I'm really not comfortable messing with it this close to the release (particularly after Jesse found a problem with the seemingly innocent change to the multiprocessing implementation in issue 3589). Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia --------------------------------------------------------------- http://www.boredomandlaziness.org
On Tue, Sep 2, 2008 at 10:03 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:
Antoine Pitrou wrote:
Nick Coghlan <ncoghlan <at> gmail.com> writes:
Given the proximity of RC1, Antoine's option 3 (leaving the capitalised factory functions in both multiprocessing and threading APIs) is actually sounding pretty appealing to me at the moment.
I was actually suggesting this course for the threading module, whose API has existed for a lot of time now, but I think it would be better to clean up multiprocessing before its first stable relase. But the issue of time and manpower starts being a bit critical :)
Unfortunately, that's where the whole "close to a drop-in replacement for threading" concept brings additions to the threading module API back into play.
If I'd realised this a bit sooner I probably would have been pushing for it to be dealt with for 2.6/3.0, but given that it's the kind of change that we can easily do through the normal API deprecation process, I'm really not comfortable messing with it this close to the release (particularly after Jesse found a problem with the seemingly innocent change to the multiprocessing implementation in issue 3589).
Cheers, Nick.
Yes, the innocuous change in 3589 - which really made a lot of sense, introduced a bug that didn't get caught until a complete make distclean; rebuild - that actually scared me off of the idea of addressing 3589 right now. I would move 3589 to 2.7/3.1 and file an additional bug for any further pep8-ifying to both the threading and mp APIs against 2.7 and 3.1 -jesse
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Jean-Paul Calderone wrote:
Here's a complaint. It's surprising that you can't use Event et al with isinstance. This is something I'm sure a lot of people run into (I did, many years ago) when they start to use these APIs. Once you do figure out why it doesn't work, it's not clear how to do what you want, since _Event is private.
I have similar issues with bsddb. Here, there are Factory functions like "DBEnv" or "DB", creating "DBEnv" and "DB" class instances. Note the name aliasing. I don't understand why these functions exist. It causes a few problems, like being unable to subclass the bsddb classes, for example. For future bsddb4.6.4 (Late october, I think) I plan to remove the Factory functions, exporting the classes directly, and allowing subclassing. The code should be 100% compatible. Any opinion? - -- Jesus Cea Avion _/_/ _/_/_/ _/_/_/ jcea@jcea.es - http://www.jcea.es/ _/_/ _/_/ _/_/ _/_/ _/_/ jabber / xmpp:jcea@jabber.org _/_/ _/_/ _/_/_/_/_/ . _/_/ _/_/ _/_/ _/_/ _/_/ "Things are not so easy" _/_/ _/_/ _/_/ _/_/ _/_/ _/_/ "My name is Dump, Core Dump" _/_/_/ _/_/_/ _/_/ _/_/ "El amor es poner tu felicidad en la felicidad de otro" - Leibniz -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.8 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iQCUAwUBSL2D3Jlgi5GaxT1NAQLJugP4qynGMZI8nN06rDyPU8FQ2kHig6uReuSE GW2fTuKXrYLlwRW5vA1GV/nA1y+6dUPuOF5erwCjVsXp28jMKNlk0BfIXmqe1wz9 +N6bIVYlFeChp5M05TDYaCNUNgRGuHURV44DvZ+vjr9GqxHuVWHHcl0EKTTSlpMi K6FBYiZjbw== =DGEq -----END PGP SIGNATURE-----
Antoine Pitrou wrote:
I don't see a problem for trivial functional wrappers to classes to be capitalized like classes.
The problem is that the capitalization makes you think it's a class, suggesting you can do things with it that you actually can't, e.g. subclassing. I can't think of any reason to do this. If you don't want to promise that something is a class, what possible reason is there for naming it like one? I can see a reason for doing the opposite, though: if something happens to be a class, but you don't want to promise that, you could expose it under a lower-case name, that can be replaced with a factory function later. In this case, the thing to decide is whether Event will always be a direct class instantiation. If so, rename _Event to Event and expose it directly. If not, rename Event to event. -- Greg
At 1:04 PM +1200 9/2/08, Greg Ewing wrote:
Antoine Pitrou wrote:
I don't see a problem for trivial functional wrappers to classes to be capitalized like classes.
The problem is that the capitalization makes you think it's a class, suggesting you can do things with it that you actually can't, e.g. subclassing.
Or that it returns a new object of that kind.
I can't think of any reason to do this. If you don't want to promise that something is a class, what possible reason is there for naming it like one? ...
Lower-case names return something about an object. Capitalized names return a new object of the named type (more or less), either via a Class constructor or a Factory object. That's /a/ reason, anyway. I suppose the question is what a capitalized name promises. If it means only "Class", then how should "Returns a new object", either from a Class or a Factory, be shown? Perhaps a new convention is needed for Factories? -- ____________________________________________________________________ TonyN.:' <mailto:tonynelson@georgeanelson.com> ' <http://www.georgeanelson.com/>
Tony Nelson wrote:
I suppose the question is what a capitalized name promises. If it means only "Class", then how should "Returns a new object", either from a Class or a Factory, be shown? Perhaps a new convention is needed for Factories?
Any function can always return a new object (e.g. operator.add(list1, list2), so I don't think we need a special naming convention to explicitly flag factory functions. The question I am raising is whether or not aberrations in the other direction (factory functions that are named like a class, incorrectly implying they can be used as a base class or as part of isinstance() or issubclass() checks) are enough of a concern to add additional aliases to the threading API, and to further modify the multiprocessing API this close to RC1. (Issue 3352 actually provides a complete list of the names that are potentially at issue for both multiprocessing and threading - note that Ben, with my concurrence, has closed that issue on the assumption that the current naming scheme for these factory functions is acceptable). Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia --------------------------------------------------------------- http://www.boredomandlaziness.org
Tony Nelson wrote:
I suppose the question is what a capitalized name promises. If it means only "Class", then how should "Returns a new object", either from a Class or a Factory, be shown?
Is there really a strong need to show that? There are many ways in which functions could be categorized. Is this particular one so important that it warrants a naming convention? -- Greg
On Mon, 1 Sep 2008 11:24:02 pm Nick Coghlan wrote:
I've been taking a close look at the API for multiprocessing and threading, and have discovered a somewhat strange pattern that occurs multiple times in both interfaces: factory functions with names that start with a capital letter so they look like they're actually a class.
At first I thought it was a quirk of the multiprocessing implementation, but then I discovered that the threading module also does it for all of the synchronisation objects as well as threading.Timer, with examples like:
def Event(*args, **kwds): return _Event(*args, **kwds)
Can anyone explain why those factory functions exist at all? They don't seem to do anything. Why not expose the class directly, instead of making it private and then exposing it via a factory function that does nothing else? Never mind whether or not the factory functions should start with an uppercase letter or not. Why are they even there? Perhaps I'm missing some subtle (or even not-so-subtle) advantage, but it seems rather silly to me, as silly as this: f = lambda x: function(x) # better written as f = function And yet threading.py has at least six examples just like that. What am I missing? -- Steven
On Tue, Sep 2, 2008 at 9:36 AM, Steven D'Aprano <steve@pearwood.info> wrote:
On Mon, 1 Sep 2008 11:24:02 pm Nick Coghlan wrote:
I've been taking a close look at the API for multiprocessing and threading, and have discovered a somewhat strange pattern that occurs multiple times in both interfaces: factory functions with names that start with a capital letter so they look like they're actually a class.
At first I thought it was a quirk of the multiprocessing implementation, but then I discovered that the threading module also does it for all of the synchronisation objects as well as threading.Timer, with examples like:
def Event(*args, **kwds): return _Event(*args, **kwds)
Can anyone explain why those factory functions exist at all? They don't seem to do anything. Why not expose the class directly, instead of making it private and then exposing it via a factory function that does nothing else? Never mind whether or not the factory functions should start with an uppercase letter or not. Why are they even there?
Perhaps I'm missing some subtle (or even not-so-subtle) advantage, but it seems rather silly to me, as silly as this:
f = lambda x: function(x) # better written as f = function
And yet threading.py has at least six examples just like that. What am I missing?
I take full responsibility. This started out as an experiment in API design, where I tried to make things look as much like the similar Java API as possible (I didn't want to invent yet anotherwobbly wheel). I specifically wanted these *not* to be classes so that people wouldn't start subclassing them. At the time PEP-8 wasn't well established (if at all) and I wanted the factory functions to look like classes. I think in 2.7 / 3.1 we can change the factory functions to conform to PEP-8 (leaving the old names in for a couple of release). I still want the classes to be private, so that it's safe to replace them with more efficient implementations on some platforms without having to worry about subclasses. -- --Guido van Rossum (home page: http://www.python.org/~guido/)
Steven D'Aprano wrote:
Why not expose the class directly, instead of making it private and then exposing it via a factory function that does nothing else?
This option would also have the advantage of not changing the API (unless there's code out there that actually depends on them *not* being classes, and that seems rather unlikely). -- Greg
Greg Ewing wrote:
Steven D'Aprano wrote:
Why not expose the class directly, instead of making it private and then exposing it via a factory function that does nothing else?
This option would also have the advantage of not changing the API (unless there's code out there that actually depends on them *not* being classes, and that seems rather unlikely).
The multiprocessing implementation currently appears to have an annoying circular import involving the multiprocessing package and the _multiprocessing extension module - the factory functions in multiprocessing.__init__ currently delay some of the relevant imports long enough to keep any problems from actually occurring. At some point that circular import should probably be eliminated (e.g. via a _mp_bootstrap module), but until that happens at least some of the factory functions in multiprocessing are actually required in order for it to be possible to import the package at all. And, as Guido already noted, leaving them as factory functions with private implementations means users can't come to rely on the objects being subclassable, and thus potentially allows more efficient implementations (e.g. using near native locking implementations directly). The only unfortunate aspect of the API is that the current names suggest that these are factory functions rather than classes - if it hadn't been for that, the question would have never even been raised. Since we already have guidelines in place for dealing with ordinary API renames, this is no longer something that strikes me as particularly urgent in the 2.6/3.0 timeframe. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia --------------------------------------------------------------- http://www.boredomandlaziness.org
2008/9/2 Greg Ewing <greg.ewing@canterbury.ac.nz>:
Steven D'Aprano wrote:
Why not expose the class directly, instead of making it private and then
exposing it via a factory function that does nothing else?
This option would also have the advantage of not changing the API (unless there's code out there that actually depends on them *not* being classes, and that seems rather unlikely).
No. Allowing them to be subclassed makes it harder to replace them on some platforms with equivalent but faster implementations. -- --Guido van Rossum (home page: http://www.python.org/~guido/)
participants (11)
-
Antoine Pitrou -
Benjamin Peterson -
Fredrik Lundh -
Greg Ewing -
Guido van Rossum -
Jean-Paul Calderone -
Jesse Noller -
Jesus Cea -
Nick Coghlan -
Steven D'Aprano -
Tony Nelson