The "PEP 8 compliant" API for multiprocessing and threading needs to be
cleaned up before release as per the thread on python-dev. The release
manager gave approval for this change during that discussion [1].

Changes needed:
- remove Py3k warnings from the threading module (old API is going to be
kept around even in Py3k)
- change get_name/set_name into a "name" property
- change is_daemon/set_daemon into a "daemon" property

(Note that changing "is_alive" to be a property is not on that list -
that can be a fairly expensive thing to check for the multiprocessing
library. I also left out active_count(), as I think the underscore adds
clarity in that case)

[1] http://mail.python.org/pipermail/python-dev/2008-June/080285.html

Does this mean that all multiprocessing get_*/set_* methods should be
changed to be properties?
For example, multiprocessing.process.Process class defines not only
set_name/get_name and is_daemon/set_daemon methods, but also
get/set_authkey, get/set_exitcode and get_ident (this method is later
used as 'getx' argument of 'pid' property, so we could get rid of this
using simple decorator).

Actually, 'getx' -> 'fget'. Sorry for the typo. :)

I think we should still keep the py3k warnings on (I'll change them to
PendingDeprecationWarnings if you want.), so the API doesn't abruptly
change on people. With your approval, I'll make it so.

At this stage I'm still inclined to skip the warnings completely - at
the very least, any eventual removals will go through a full deprecation
cycle in 2.7/3.1 before being removed in 2.8/3.2.

It's also much easier to be sure we aren't adversely affecting
performance of the old methods that way (with a
PendingDeprecationWarning, performance of the deprecated methods is
going to suffer a bit since the warnings machinery will still have to be
invoked to discover that the warning has been filtered out). That said,
the methods we're changing aren't likely to be something one invokes in
speed critical sections of an application, so I wouldn't object if the
old names emitted PDW in both 2.6 and 3.0.

Regarding the additional get/set properties on multiprocessing.Process,
it depends on whether or not they are cheap to calculate and whether or
not they involve any I/O operations. auth_key looked like it may be
expensive to set (since I assume there is a crypto calculation involved
there), exit_code may involve waiting for the other process to finish,
and get_ident may involve waiting for it to start. So my initial
reaction is that these are OK to keep as methods rather than trying to
turn them into properties.

The following pretty undesirable behaviour should probably be discussed
on python-dev before beta3 (if not beta2):

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)

Is anybody working on a patch for this?  Nick, I agree with you about
undesirable behavior, however if there is no patch currently under
development, I'm inclined to defer blocking until beta3.

I don't want to change the API or any other code before we get the change 
in for issue874900 which should fix/resolve issue3088

On Jul 16, 2008, at 9:12 AM, Jesse Noller wrote:

> I don't want to change the API or any other code before we get the  
> change
> in for issue874900 which should fix/resolve issue3088

What's the holdup on 874900?

I don't know, but I am going to ping Antoine and Greg and see if they 
don't mind me applying it as-is.

This should not block the release of beta 2 IMHO

Sorry, I'm not going to be able to work on this soon.

I created issue 3589 to cover the problem I mentioned above with the
very misleading names for the package level functions in multiprocessing.

I think I can get this done before the release now. For starters I
changed thread.get_ident() to a property in r65818.

Ok. I've done the threading API and patched up multiprocessing so it
actually works. Jesse, can you do multiprocessing?

Also, can we decide about is_alive as a property?

Oh, and by the way, I would start working on multiprocessing by
reverting r65828.

On Mon, Aug 18, 2008 at 2:44 PM, Benjamin Peterson
<report@bugs.python.org> wrote:
>
> Benjamin Peterson <musiccomposition@gmail.com> added the comment:
>
> Ok. I've done the threading API and patched up multiprocessing so it
> actually works. Jesse, can you do multiprocessing?
>
> Also, can we decide about is_alive as a property?
>

Yup, I can do the MP part.

On Mon, Aug 18, 2008 at 2:45 PM, Benjamin Peterson
<report@bugs.python.org> wrote:
>
> Benjamin Peterson <musiccomposition@gmail.com> added the comment:
>
> Oh, and by the way, I would start working on multiprocessing by
> reverting r65828.
>

Makes sense.

is_alive appears to be a potentially expensive check for the
multiprocessing side of things, which is why I'm inclined to leave it as
a method. "if t.is_alive():" actually reads better to me than "if
t.alive:" anyway.

On Tue, Aug 19, 2008 at 10:34 AM, Nick Coghlan <report@bugs.python.org> wrote:
>
> Nick Coghlan <ncoghlan@gmail.com> added the comment:
>
> is_alive appears to be a potentially expensive check for the
> multiprocessing side of things, which is why I'm inclined to leave it as
> a method. "if t.is_alive():" actually reads better to me than "if
> t.alive:" anyway.

So be it. I don't really want to do any renaming anyway. :)
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue3352>
> _______________________________________
>

-- 
Cheers,
Benjamin Peterson
"There's no place like 127.0.0.1."

On Tue, Aug 19, 2008 at 11:34 AM, Nick Coghlan <report@bugs.python.org> wrote:
>
> Nick Coghlan <ncoghlan@gmail.com> added the comment:
>
> is_alive appears to be a potentially expensive check for the
> multiprocessing side of things, which is why I'm inclined to leave it as
> a method. "if t.is_alive():" actually reads better to me than "if
> t.alive:" anyway.

Dang, I already cut that one over.

My preference actually isn't particularly strong either way, so I
suggest asking Barry for a casting vote.

Nick/Ben - here's a rough draft patch for the attribute changes for mp - 
could you both take a look and let me know what you think?

The patch looks pretty good. In a few places, you replaced x.set_x(True)
with x.x(True). The docs also will need a few tweaks. Otherwise, if the
tests pass, I say go ahead.

Just saw a couple of docs checkins go by suggesting that the camelCase
names will be unavailable in 3.x. Remember that the intent *isn't* to
remove the old names from threading.py - we're just adding the PEP 8
names so that multiprocessing can have a PEP 8 compliant API while still
being close to a drop-in replacement for the 2.6 version of the
threading module.

This means that all of the DeprecationWarnings and so forth can be left
out completely.

Regarding your patch Jesse:
- if I remember rightly, get_exitcode() can block waiting for the other
process to finish, which makes turning it into a property a fairly
questionable idea (p.get_exitcode() blocking is a lot less surprising
than "p.exitcode" doing so)
- the get/set documentation for the new properties can probably be
consolidated, but that isn't a major problem as far as this week's beta
is concerned.

Note regarding those comments - only the exitcode one is something we
should try to get sorted for the beta. Backing out the deprecation
warnings and cleaning up the documentation can wait for the first
release candidate.

Barry said that is_alive should remain a method following in Guido's
footsteps.

>
> Nick Coghlan <ncoghlan@gmail.com> added the comment:
>
> Note regarding those comments - only the exitcode one is something we
> should try to get sorted for the beta. Backing out the deprecation
> warnings and cleaning up the documentation can wait for the first
> release candidate.
>

Would it be acceptable to add a doc note mentioning .exitcode() may
block rather than changing it back to the get_exitcode method? It's
not that it's hard - but I do like the consistency of the
property-style interface.

>
> Nick Coghlan <ncoghlan@gmail.com> added the comment:
>
> Note regarding those comments - only the exitcode one is something we
> should try to get sorted for the beta. Backing out the deprecation
> warnings and cleaning up the documentation can wait for the first
> release candidate.
>

Actually, re-examining .exitcode - it wraps the custom forking.Popen
class .poll() method, this method should not block (forking.py, line
104) - the WNOHANG call passed to os.waitpid means it should also not
hang.

Here is a cleaned up patch (ben was right, I typo'ed a few spots) - I did 
not clean up the docs from the original patch except to correct the 
x.x(Foo) accidents.

I attached the wrong patch, here is v2

And here is v3 - I fixed all of the Docs/include/mp* scripts to reflect 
the new API

committed the v3 patch as-is in r65864 - moving from release blocker to 
deferred blocker pending the Docs cleanup, and outstanding debate points.

There are still some instances of get_name() in threading.py itself,
which gives errors like the following:


Unhandled exception in thread started by <bound method
Thread.__bootstrap of <Thread(reader 5, stopped daemon -1285227632)>>
Traceback (most recent call last):
  File "/home/antoine/cpython/cpickle/Lib/threading.py", line 499, in
__bootstrap
    self.__bootstrap_inner()
  File "/home/antoine/cpython/cpickle/Lib/threading.py", line 537, in
__bootstrap_inner
    (self.get_name(), _format_exc()))
AttributeError: 'Thread' object has no attribute 'get_name'

Here's a patch.

I found further PEP 8 non-compliances in the multiprocessing API while
working on a patch for issue 3589, mainly in the area of function names
that start with a capital letter, making them look like classes when
they definitely are not.

After noticing a few of these, I went through checked more thoroughly,
and found all of the following to be functions that claimed to be
classes by way of their naming convention (a far worse sin than using
camelCase instead of underscores):
multiprocessing.Pipe (aka multiprocessing.connection.Pipe)
multiprocessing.RawValue (aka multiprocessing.sharedctypes.RawValue)
multiprocessing.RawArray (aka multiprocessing.sharedctypes.RawArray)
multiprocessing.Value (aka multiprocessing.sharedctypes.Value)
multiprocessing.Array (aka multiprocessing.sharedctypes.Array)
multiprocessing.connection.Client
multiprocessing.connection.SocketClient
multiprocessing.connection.PipeClient
multiprocessing.connection.XmlClient
multiprocessing.managers.RebuildProxy
multiprocessing.managers.MakeProxyType
multiprocessing.managers.AutoProxy
multiprocessing.managers.Array

These should all be converted to start with a lowercase letter and use
underscores, otherwise people are going to assume they can be treated
like classes.

Benjamin's patch was applied in r65982

Patch added that removes the incorrect Py3k warnings from the threading
module (also restores the methods to the same __name__ attributes as
they had in 2.5).

Second patch added that removes the deprecation warnings from the Py3k
version of the threading module.

It turns out threading uses the odd "class-that-is-not-a-class" naming
scheme as well:
threading.Lock
threading.RLock
threading.Condition
threading.Semaphore
threading.BoundedSemaphore
threading.Event
threading.Timer

Patch added to tone down note regarding the PEP 8 compliant aliases that
have been added to the threading module.

And one last patch to adjust the threading docs in Py3k to reflect the
fact that the 2.x API is still supported, even if it is no longer
documented.

Updated the 2.6 threading patch to also remove the warnings from the
methods that are being replaced by properties.

The patches look good to me. Please apply.

Regarding the factory functions that are named as if they were classes,
Fredrik noted on python-dev that the ones from the threading module are
explicitly documented as being factory functions, and the
multiprocessing API really just follows that example (note that without
applying the patch from issue 3589, all of the names that are factory
functions in the threading API are also factory functions in the
multiprocessing API).

So perhaps the best course at this stage is to just leave these alone
for 2.6/3.0?

Ben, if you get a chance to apply those patches, feel free, otherwise I
should be able to get to them this evening (my time - about 10 hours
from now).

Ok. patches applied in r66126 and r66127.

Thanks!