msg71327 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2008-08-18 13:27 |
The package level imports from the new multiprocessing package exhibit
some very strange behaviour because they are actually functions
pretending to be classes:
Python 2.6b1+ (trunk:64945, Jul 14 2008, 20:00:46)
[GCC 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import multiprocessing as mp
>>> isinstance(mp.Lock(), mp.Lock)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes
and types
>>> mp.Lock.__name__
'Lock'
>>> mp.Lock.__module__
'multiprocessing'
>>> mp.Lock().__class__.__name__
'Lock'
>>> mp.Lock().__class__.__module__
'multiprocessing.synchronize'
The delayed import functions in multiprocessing.__init__ look like a
serious misfeature to me. I'd be inclined to replace them with "from
.synchronize import *" and "from .process import *" (leaving anything
which isn't covered by those two imports to be retrieved directly from
the relevant mp submodule)
|
msg71328 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2008-08-18 13:28 |
Setting to deferred blocker, since this really needs to be dealt with
for RC1 (probably too close to b3 to get it discussed and dealt with for
that).
|
msg71334 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2008-08-18 14:30 |
> The delayed import functions in multiprocessing.__init__ look like a
> serious misfeature to me. I'd be inclined to replace them with "from
> .synchronize import *" and "from .process import *"
+1
(or, even better, qualified than wildcard imports)
|
msg72228 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2008-09-01 11:57 |
Patch attached that removes the misleading "convenience" functions,
replacing them with explicit imports of the appropriate names.
The patch also adds docstrings to some of the original class definitions
that were missing them.
No changes were needed to the multiprocessing tests - they all still
passed with this change, and the docs are still accurate as well (I
would actually say this change makes the docs MORE accurate).
Python 2.6b3+ (trunk:66083, Aug 31 2008, 19:00:32)
[GCC 4.2.3 (Ubuntu 4.2.3-2ubuntu7)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import multiprocessing as mp
>>> isinstance(mp.Lock(), mp.Lock)
True
>>> mp.Lock.__name__
'Lock'
>>> mp.Lock.__module__
'multiprocessing.synchronize'
|
msg72237 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2008-09-01 13:07 |
Interesting - in some of the other work I was doing regarding the PEP 8
compliant alternative threading API, I noticed that the threading module
contains similar gems such as:
def Event(*args, **kwds):
return _Event(*args, **kwds)
Using a factory function to discourage subclassing is one thing, but why
name the factory function as if it was a still a class?
|
msg72244 - (view) |
Author: Jesse Noller (jnoller) * |
Date: 2008-09-01 14:03 |
This is why multiprocessing had them nick - the threading module does
On Sep 1, 2008, at 9:07 AM, Nick Coghlan <report@bugs.python.org> wrote:
>
> Nick Coghlan <ncoghlan@gmail.com> added the comment:
>
> Interesting - in some of the other work I was doing regarding the
> PEP 8
> compliant alternative threading API, I noticed that the threading
> module
> contains similar gems such as:
>
> def Event(*args, **kwds):
> return _Event(*args, **kwds)
>
> Using a factory function to discourage subclassing is one thing, but
> why
> name the factory function as if it was a still a class?
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue3589>
> _______________________________________
|
msg72274 - (view) |
Author: Jesse Noller (jnoller) * |
Date: 2008-09-01 16:47 |
Patch reviewed/tested and I also confirmed that this doesn't affect the
examples. I submitted the patch in r66114
|
msg72277 - (view) |
Author: Jesse Noller (jnoller) * |
Date: 2008-09-01 17:09 |
Reopening, there's a bug that the tests/examples/etc didn't catch (and
nor did I), after the patch application:
woot:python-trunk jesse$ ./python.exe
Python 2.6b3+ (trunk:66112:66114M, Sep 1 2008, 13:00:19)
[GCC 4.0.1 (Apple Inc. build 5484)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import _multiprocessing
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/jesse/open_source/subversion/python-
trunk/Lib/multiprocessing/__init__.py", line 148, in <module>
from multiprocessing.synchronize import (Lock, RLock, Condition,
Event,
File "/Users/jesse/open_source/subversion/python-
trunk/Lib/multiprocessing/synchronize.py", line 29, in <module>
SEM_VALUE_MAX = _multiprocessing.SemLock.SEM_VALUE_MAX
AttributeError: 'module' object has no attribute 'SemLock'
>>>
|
msg72278 - (view) |
Author: Jesse Noller (jnoller) * |
Date: 2008-09-01 17:09 |
Ben is backing out the patch now
|
msg72294 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2008-09-01 21:22 |
Given how long I've been using the threading module without realising it
does the same thing, I'm actually prepared to live with the wrapper
functions rather than messing with this so close to release.
As Fredrik noted in the python-dev thread, the threading versions of
these are already explicitly documented as being factory functions
rather than classes (and as a reference to _thread.allocate_lock,
threading.Lock has good reason to be a factory function rather than a
class), so it may be appropriate to do the same thing for multiprocessing.
|
msg72297 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2008-09-01 21:31 |
Sounds good to me. :)
|
msg72303 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2008-09-01 22:11 |
Le lundi 01 septembre 2008 à 21:22 +0000, Nick Coghlan a écrit :
> As Fredrik noted in the python-dev thread, the threading versions of
> these are already explicitly documented as being factory functions
> rather than classes (and as a reference to _thread.allocate_lock,
> threading.Lock has good reason to be a factory function rather than a
> class), so it may be appropriate to do the same thing for multiprocessing.
"Foolish consistency, etc.". :-)
I think we should fix this before the releases rather than live with a
stupid indirection that doesn't seem to serve any useful purposes.
Unless the fix is too complicated of course.
|
msg72338 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2008-09-02 14:02 |
Not so much "too complicated" as "potentially touches a lot of code and
not something I want to fiddle with this close to release".
As I noted on python-dev, it's actually a change that can easily be
handled through the normal API deprecation process, so it can be
reconsidered at a later date.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:37 | admin | set | nosy:
+ barry github: 47839
|
2008-09-02 14:02:21 | ncoghlan | set | messages:
+ msg72338 |
2008-09-01 22:11:25 | pitrou | set | messages:
+ msg72303 |
2008-09-01 21:31:44 | benjamin.peterson | set | status: open -> closed resolution: wont fix messages:
+ msg72297 nosy:
+ benjamin.peterson |
2008-09-01 21:22:16 | ncoghlan | set | messages:
+ msg72294 |
2008-09-01 17:09:19 | jnoller | set | messages:
+ msg72278 |
2008-09-01 17:09:03 | jnoller | set | status: closed -> open resolution: fixed -> (no value) messages:
+ msg72277 |
2008-09-01 16:47:53 | jnoller | set | status: open -> closed resolution: fixed messages:
+ msg72274 |
2008-09-01 16:45:59 | jnoller | set | keywords:
+ needs review, - patch |
2008-09-01 14:03:00 | jnoller | set | messages:
+ msg72244 |
2008-09-01 13:07:52 | ncoghlan | set | messages:
+ msg72237 |
2008-09-01 11:57:53 | ncoghlan | set | files:
+ issue3589_true_aliases_in_multiprocessing.diff keywords:
+ patch messages:
+ msg72228 |
2008-08-21 03:06:26 | barry | set | priority: deferred blocker -> release blocker |
2008-08-18 14:30:41 | pitrou | set | nosy:
+ pitrou messages:
+ msg71334 |
2008-08-18 13:28:25 | ncoghlan | set | priority: critical -> deferred blocker messages:
+ msg71328 |
2008-08-18 13:27:13 | ncoghlan | create | |