Re: [Python-Dev] Asking for reversion

[Antoine Pitrou]

On Sat, 23 Feb 2019 22:09:03 -0600
Davin Potts  wrote:
> I have done what I was asked to do:  I added tests and docs in a new
> PR (GH-11816) as of Feb 10.
> 
> Since that time, the API has matured thanks to thoughtful feedback
> from a number of active reviewers.  At present, we appear to have
> stabilized around an API and code that deserves to be exercised
> further.  To get that exercise and because there are currently no
> outstanding objections, I am merging the PR to get it into the second
> alpha.  There will undoubtedly be further revisions and adjustments.

I agree the overall API looks reasonable.  Thanks for doing this in
time.

Regards

Antoine.


___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Giampaolo Rodola']

On Sun, Feb 24, 2019 at 5:09 AM Davin Potts <
python+python_...@discontinuity.net> wrote:

> I have done what I was asked to do:  I added tests and docs in a new
> PR (GH-11816) as of Feb 10.
>
> Since that time, the API has matured thanks to thoughtful feedback
> from a number of active reviewers.  At present, we appear to have
> stabilized around an API and code that deserves to be exercised
> further.  To get that exercise and because there are currently no
> outstanding objections, I am merging the PR to get it into the second
> alpha.  There will undoubtedly be further revisions and adjustments.


Nice job! It wasn't easy to abstract such a low level interface.

-- 
Giampaolo - http://grodola.blogspot.com
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Davin Potts]

I have done what I was asked to do:  I added tests and docs in a new
PR (GH-11816) as of Feb 10.

Since that time, the API has matured thanks to thoughtful feedback
from a number of active reviewers.  At present, we appear to have
stabilized around an API and code that deserves to be exercised
further.  To get that exercise and because there are currently no
outstanding objections, I am merging the PR to get it into the second
alpha.  There will undoubtedly be further revisions and adjustments.


During this effort, I have received a surprising number of personal
emails expressing support and encouragement from people, most of whom
I have never met.  Your kindness has been wonderful.


Davin

On Wed, Feb 6, 2019 at 10:58 AM Giampaolo Rodola'  wrote:
>
>
>
> On Wed, Feb 6, 2019 at 12:51 PM Giampaolo Rodola'  wrote:
>>
>>
>> Unless they are already there (I don't know) it would be good to have a full 
>> set of unit-tests for all the register()ed types and test them against 
>> SyncManager and SharedMemoryManager. That would give an idea on the real 
>> interchangeability of these 2 classes and would also help writing a 
>> comprehensive doc.
>
>
> In order to speed up the alpha2 inclusion process I created a PR which 
> implements what said above:
> https://github.com/python/cpython/pull/11772
> https://bugs.python.org/issue35917
> Apparently SharedMemoryManager works out of the box and presents no 
> differences with SyncManager, but the list type is not using ShareableList. 
> When I attempted to register it with "SharedMemoryManager.register('list', 
> list, ShareableList)" I got the following error:
>
> Traceback (most recent call last):
>   File "foo.py", line 137, in test_list
> o = self.manager.list()
>   File "/home/giampaolo/svn/cpython/Lib/multiprocessing/managers.py", line 
> 702, in temp
> proxy = proxytype(
> TypeError: __init__() got an unexpected keyword argument 'manager'
>
> I am not sure how to fix that (I'll leave it up to Davin). The tests as-is 
> are independent from PR-11772 so I suppose they can be reviewed/checked-in 
> regardless of the changes which will affect shared_memory.py.
>
> --
> Giampaolo - http://grodola.blogspot.com
>
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Giampaolo Rodola']

On Wed, Feb 6, 2019 at 12:51 PM Giampaolo Rodola' 
wrote:

>
> Unless they are already there (I don't know) it would be good to have a
> full set of unit-tests for all the register()ed types and test them against
> SyncManager and SharedMemoryManager. That would give an idea on the real
> interchangeability of these 2 classes and would also help writing a
> comprehensive doc.
>

In order to speed up the alpha2 inclusion process I created a PR which
implements what said above:
https://github.com/python/cpython/pull/11772
https://bugs.python.org/issue35917
Apparently SharedMemoryManager works out of the box and presents no
differences with SyncManager, but the list type is not using ShareableList.
When I attempted to register it with "SharedMemoryManager.register('list',
list, ShareableList)" I got the following error:

Traceback (most recent call last):
  File "foo.py", line 137, in test_list
o = self.manager.list()
  File "/home/giampaolo/svn/cpython/Lib/multiprocessing/managers.py", line
702, in temp
proxy = proxytype(
TypeError: __init__() got an unexpected keyword argument 'manager'

I am not sure how to fix that (I'll leave it up to Davin). The tests as-is
are independent from PR-11772 so I suppose they can be reviewed/checked-in
regardless of the changes which will affect shared_memory.py.

-- 
Giampaolo - http://grodola.blogspot.com
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Giampaolo Rodola']

Davin,
I am not familiar with the multiprocessing module, so take the following
with a big grain of salt. I took a look at the PR, then I got an idea of
how multiprocessing module is organized by reading the doc. Here's some
food for thought in terms of API reorganization.

SharedMemoryManager, SharedMemoryServer
---

It appears to me these are the 2 main public classes, and after reading the
doc it seems they really belong to "managers
"
(multiprocessing.managers namespace). Also:
* SharedMemoryManager is a subclass of multiprocessing.managers.SyncManager
* SharedMemoryServer is a subclass of multiprocessing.managers.Server
shared_memory.py could be renamed to _shared_memory.py and managers.py
could import and expose these 2 classes only.

Support APIs


These are objects which seem to be used in support of the 2 classes above,
but apparently are not meant to be public. As such they could simply live
in _shared_memory.py and not be exposed:

- shareable_wrap(): used only in SharedMemoryTracker.wrap()
- SharedMemoryTracker: used only by SharedMemoryServer
- SharedMemory, WindowsNamedSharedMemory, PosixSharedMemory: used by
shareable_wrap() and SharedMemoryTracker
- ShareableList: it appears this is not used, but by reading here

I have a doubt: shouldn't it be register()ed against SharedMemoryManager?

C extension module
--

- ExistentialError, Error - it appears these are not used
- PermissionsException, ExistentialException - I concur with Ronald
Oussoren's review: you could simply use PyErr_SetFromErrno() and let the
original OSError exception bubble up. Same for O_CREAT, O_EXCL, O_CREX,
O_TRUNC which are already exposed in the os module. I have a couple of
other minor nitpicks re. the code but I will comment on the PR.

Compatibility
-

I'm not sure if SyncManager and SharedMemoryManager are fully
interchangeable so I think the doc should clarify this. SyncManager handles
a certain set of types
.
It appears SharedMemoryManager is supposedly able to do the same except for
lists
.
Is my assumption correct? Also, multiprocessing.Manager() by default
returns a SyncManager. If we'll get to a point where SyncManager and
SharedMemoryManager are able to handle the same types it'd be good to
return SharedMemoryManager as the default, but it's probably safer to leave
it for later. Unless they are already there (I don't know) it would be good
to have a full set of unit-tests for all the register()ed types and test
them against SyncManager and SharedMemoryManager. That would give an idea
on the real interchangeability of these 2 classes and would also help
writing a comprehensive doc.

Hope this helps.

-- 
Giampaolo - http://grodola.blogspot.com
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Nick Coghlan]

On Wed, 6 Feb 2019 at 05:17, Neil Schemenauer  wrote:
> My gut reaction is that we shouldn't revert.  However, looking at
> the changes, it seems 'multiprocessing.shared_memory' could be an
> external extension package that lives in PyPI.  It doesn't require
> changes to other interpreter internals.  It doesn't seem to require
> internal Python header files.

The desired dependency in this case goes the other way: we'd like this
in the standard library so that other standard library components can
use it, and it can eventually become part of the "assumed baseline"
that the reference Python interpreter offers to projects building on
top of it.

Cheers,
Nick.

-- 
Nick Coghlan   |   ncogh...@gmail.com   |   Brisbane, Australia
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Ethan Furman]

On 02/05/2019 11:35 AM, Raymond Hettinger wrote:


How about we stop using a highly public forum to pile up on Davin (being the 
subject of a thread like this can be a soul crushing experience).


Thank you for the reminder.


 Right now, he could really use some help and support from everyone on the team.


I am really looking forward to this enhancement.  Thank you, Davin, and 
everyone else who has, and will, work on it.

--
~Ethan~
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Neil Schemenauer]

I wrote:
> Could we somehow mark these APIs as experimental in 3.8?

It seems the change "e5ef45b8f519a9be9965590e1a0a587ff584c180" the
one we are discussing.  It adds two new files:

  Lib/multiprocessing/shared_memory.py
  Modules/_multiprocessing/posixshmem.c

It doesn't introduce new C APIs.  So, only
multiprocessing.shared_memory seems public.  I see we have PEP 411
that should cover this case:

  https://www.python.org/dev/peps/pep-0411/

The setup.py code could be more defensive.  Maybe only build on
platforms that have supported word sizes etc?  For 3.8, could it be
activated by uncommenting a line in Modules/Setup, rather than by
setup.py?

What happens in shared_memory if the _posixshmem module is not
available?  On Windows it seems like an import error is raised.
Otherwise, _PosixSharedMemory becomes 'object'.  Does that mean the
API still works but you lose the zero-copy speed?

Regards,

  Neil
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Raymond Hettinger]

> On Feb 5, 2019, at 9:52 AM, Giampaolo Rodola'  wrote:
> 
>  The main problem I have with this PR is that it seems to introduce 8 brand 
> new APIs, but since there is no doc, docstrings or tests it's unclear which 
> ones are supposed to be used, how or whether they are supposed to supersede 
> or deprecate older (slower) ones involving inter process communication.

The release manger already opined that if tests and docs get finished for the 
second alpha, he prefers not to have a reversion and would rather on build on 
top of what already shipped in the first alpha.  FWIW, the absence of docs 
isn't desirable but it isn't atypical.  PEP 572 code landed without the docs. 
Docs for dataclasses arrived much after the code. The same was true for the 
decimal module. Hopefully, everyone will team up with Davin and help him get 
the ball over the goal line.

BTW, this is a feature we really want.  Our multicore story for Python isn't a 
good one.  Due to the GIL, threading usually can't exploit multiple cores for 
better performance.  Async has lower overhead than threading but achieves its 
gains by keeping all the data in a single process.  That leaves us with 
multiprocessing where the primary obstacle has been the heavy cost of moving 
data between processes.  If that cost can be reduced, we've got a winning story 
for multicore.

This patch is one of the better things that is happening to Python.  Aside from 
last week's procedural missteps and communication issues surrounding the 
commit, the many months of prior work on this have been stellar. How about we 
stop using a highly public forum to pile up on Davin (being the subject of a 
thread like this can be a soul crushing experience).  Right now, he could 
really use some help and support from everyone on the team.


Raymond


___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Neil Schemenauer]

On 2019-02-05, Giampaolo Rodola' wrote:
> The main problem I have with this PR is that it seems to introduce
> 8 brand new APIs, but since there is no doc, docstrings or tests
> it's unclear which ones are supposed to be used, how or whether
> they are supposed to supersede or deprecate older (slower) ones
> involving inter process communication.

New or changed APIs are my major concern as well.  Localized
problems can be fixed later without much trouble.  However, APIs
"lock" us in and make it harder to change things later.  Also, will
new APIs need to be eventually supported by other Python
implementations?  I would imagine that doing zero-copy mixed with
alternative garbage collection strategies could be complicated.
Could we somehow mark these APIs as experimental in 3.8?

My gut reaction is that we shouldn't revert.  However, looking at
the changes, it seems 'multiprocessing.shared_memory' could be an
external extension package that lives in PyPI.  It doesn't require
changes to other interpreter internals.  It doesn't seem to require
internal Python header files.

Regards,

  Neil
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Giampaolo Rodola']

On Mon, Feb 4, 2019 at 4:21 AM Davin Potts <
python+python_...@discontinuity.net> wrote:

> I am attempting to do the right thing and am following the advice of other
> core devs in what I have done thus far.
>
> Borrowing heavily from what I've added to issue35813 just now:
>
> This work is the result of ~1.5 years of development effort, much of it
> accomplished at the last two core dev sprints.  The code behind it has been
> stable since September 2018 and tested as an independently installable
> package by multiple people.
>
> I was encouraged by Lukasz, Yury, and others to check in this code early,
> not waiting for tests and docs, in order to both solicit more feedback and
> provide for broader testing.  I understand that doing such a thing is not
> at all a novelty.
>

Actually it is a novelty (you should wait for review and approval). The
main problem I have with this PR is that it seems to introduce 8 brand new
APIs, but since there is no doc, docstrings or tests it's unclear which
ones are supposed to be used, how or whether they are supposed to supersede
or deprecate older (slower) ones involving inter process communication. The
introduction of new APIs in the stdlib is a sensitive topic because once
they get in they stay in, so a discussion should occur early on,
definitively not at alphaX stage. Don't mean to point fingers here, the
goal in itself (zero-copy, a topic I recently contributed to myself for the
shutil module) is certainly valuable, but I concur and think this change
should be reverted and post-poned for 3.9.

-- 
Giampaolo - http://grodola.blogspot.com
-- 
Giampaolo - http://grodola.blogspot.com
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Raymond Hettinger]

> On Feb 4, 2019, at 2:36 AM, Łukasz Langa  wrote:
> 
> @Raymond, would you be willing to work with Davin on finishing this work in 
> time for alpha2?

I would be happy to help, but this is beyond my technical ability.  The people 
who are qualified to work on this have already chimed in on the discussion.  
Fortunately, I think this is a feature that everyone wants. So it just a matter 
of getting the experts on the subject to team-up and help get it done.


Raymond





___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Eric Snow]

The main problem here seems to be a shortage of communication. :/
Also, I agree on the exceptional nature of merging incomplete PRs.

-eric

On Mon, Feb 4, 2019 at 3:37 AM Łukasz Langa  wrote:
>
>
> > On 4 Feb 2019, at 01:49, Guido van Rossum  wrote:
> >
> > I think this is now up to the 3.8 release manager.
>
> I responded on the tracker: https://bugs.python.org/issue35813#msg334817
>
> I wrote:
>
> > @Davin, in what time can you fill in the missing tests and documentation?  
> > If this is something you finish do before alpha2, I'm inclined to leave the 
> > change in.
> >
> > As it stands, I missed the controversy yesterday as I was busy making my 
> > first release.  So the merge *got released* in alpha1.  I would prefer to 
> > fix the missing pieces forward instead of reverting and re-submitting which 
> > will only thrash blame and history at this point.
> >
> > FTR, I do agree with Antoine, Ronald and others that in the future such big 
> > changes should be as close to their ready state at merge time.
>
>
>
> @Raymond, would you be willing to work with Davin on finishing this work in 
> time for alpha2?
>
>
> - Ł
> ___
> Python-Dev mailing list
> Python-Dev@python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe: 
> https://mail.python.org/mailman/options/python-dev/ericsnowcurrently%40gmail.com
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Łukasz Langa]

> On 4 Feb 2019, at 01:49, Guido van Rossum  wrote:
> 
> I think this is now up to the 3.8 release manager.

I responded on the tracker: https://bugs.python.org/issue35813#msg334817

I wrote:

> @Davin, in what time can you fill in the missing tests and documentation?  If 
> this is something you finish do before alpha2, I'm inclined to leave the 
> change in.
> 
> As it stands, I missed the controversy yesterday as I was busy making my 
> first release.  So the merge *got released* in alpha1.  I would prefer to fix 
> the missing pieces forward instead of reverting and re-submitting which will 
> only thrash blame and history at this point.
> 
> FTR, I do agree with Antoine, Ronald and others that in the future such big 
> changes should be as close to their ready state at merge time.



@Raymond, would you be willing to work with Davin on finishing this work in 
time for alpha2?


- Ł


signature.asc
Description: Message signed with OpenPGP
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Antoine Pitrou]

On Sun, 3 Feb 2019 21:12:38 -0600
Davin Potts  wrote:
> 
> I was encouraged by Lukasz, Yury, and others to check in this code early,
> not waiting for tests and docs, in order to both solicit more feedback and
> provide for broader testing.

For the record: submitting a PR without tests or docs is perfectly
fine, and a reasonable way to ask for feedback.  Merging that PR is
not, usually (especially as you didn't wait for feedback).

So there might have been a misunderstanding between you and
Lukasz, Yury and the "others".  Or perhaps this is another instance of
taking a disruptive decision in private...

Regards

Antoine.


___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Antoine Pitrou]

On Sun, 3 Feb 2019 18:10:43 -0800
Raymond Hettinger  wrote:
> > On Feb 3, 2019, at 5:40 PM, Terry Reedy  wrote:
> > 
> > On 2/3/2019 7:55 PM, Guido van Rossum wrote:  
> >> Also, did anyone ask Davin directly to roll it back?  
> > 
> > Antoine posted on the issue, along with Robert O.  Robert reviewed and make 
> > several suggestions.  
> 
> I think the PR sat in a stable state for many months, 

According to Github, it was opened 11 days ago.
The first commit itself is 12 days old (again according to Github):
https://github.com/python/cpython/pull/11664/commits/90f4a6cb2da8e187fa38b05c3f347cd602dd69c5

Now, perhaps the work itself is much older.

But regardless, you cannot expect someone to take notice of a PR or
issue if they are not put in CC.  It is very much against our usual
conventions to check in large pieces of code without asking anyone for
review.

Regards

Antoine.


___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Antoine Pitrou]

On Sun, 3 Feb 2019 17:52:55 -0800
Raymond Hettinger  wrote:
> > On Feb 3, 2019, at 1:03 PM, Antoine Pitrou  wrote:
> > 
> > I'd like to ask for the reversion of the changes done in
> > https://github.com/python/cpython/pull/11664  
> 
> Please work *with* Davin on this one.

You know, Raymond, I'm a volunteer and I dedicate my time to whatever I
want.  If someone pushes some unfinished work, it is perfectly normal
to ask for reversion instead of feeling obliged to finish the work
myself.

Regards

Antoine.
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Antoine Pitrou]

On Sun, 3 Feb 2019 21:25:27 -0600
Davin Potts  wrote:
> On 2/3/2019 7:55 PM, Guido van Rossum wrote:
> > Also, did anyone ask Davin directly to roll it back?  
> 
> Simply put:  no.  There have been a number of reactionary comments in the
> last 16 hours but no attempt to reach out to me directly during that time.

By construction, if I post a comment on an issue you opened yourself on
the bug tracker, you are receiving those comments.

I'm not sure why a private message would be necessary.  Generally, we
refrain from doing things in private except if there are personal
issues.

Regards

Antoine.


___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Ronald Oussoren via Python-Dev]

> On 4 Feb 2019, at 04:25, Davin Potts  
> wrote:
> 
> On 2/3/2019 7:55 PM, Guido van Rossum wrote:
> > Also, did anyone ask Davin directly to roll it back?
> 
> Simply put:  no.  There have been a number of reactionary comments in the 
> last 16 hours but no attempt to reach out to me directly during that time.
> 

I asked a question about the commit yesterday night in the tracker and was 
waiting for a response (which I fully expected to take some time due to 
timezone differences and this being a volunteer driven project). 

Ronald
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Ronald Oussoren via Python-Dev]

> On 4 Feb 2019, at 03:10, Raymond Hettinger  
> wrote:
> 
> 
>> On Feb 3, 2019, at 5:40 PM, Terry Reedy  wrote:
>> 
>> On 2/3/2019 7:55 PM, Guido van Rossum wrote:
>>> Also, did anyone ask Davin directly to roll it back?
>> 
>> Antoine posted on the issue, along with Robert O.  Robert reviewed and make 
>> several suggestions.

@Terry: Robert is usually called Ronald :-)

> 
> I think the PR sat in a stable state for many months, and it looks like RO's 
> review comments came *after* the commit.  

That’s because I only noticed the PR after commit: The PR was merged within an 
hour of creating the BPO issue. 

> 
> FWIW, with dataclasses we decided to get the PR committed early, long before 
> most of the tests and all of the docs. The principle was that bigger changes 
> needed to go in as early as possible in the release cycle so that we could 
> thoroughly exercise it (something that almost never happens while something 
> is in the PR stage).  It would be great if the same came happen here.  IIRC, 
> shared memory has long been the holy grail for multiprocessing, helping to 
> mitigate its principle disadvantage (the cost of moving data between 
> processes).  It's something we really want.

But with dataclasses there was public discussion on the API.  This is a new API 
with no documentation in a part of the library that is known to be complex in 
nature.

Ronald
--

Twitter: @ronaldoussoren
Blog: https://blog.ronaldoussoren.net/
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Barry Warsaw]

On Feb 3, 2019, at 18:10, Raymond Hettinger  wrote:
> 
> FWIW, with dataclasses we decided to get the PR committed early, long before 
> most of the tests and all of the docs. The principle was that bigger changes 
> needed to go in as early as possible in the release cycle so that we could 
> thoroughly exercise it (something that almost never happens while something 
> is in the PR stage).

I think that should generally be the exception, but if it does happen, there 
ought to be a release blocker issue for the tests and docs.  The problem then 
is if those things *don’t* happen and we get too late in the release cycle to 
roll the change back.

-Barry



signature.asc
Description: Message signed with OpenPGP
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Davin Potts]

On 2/3/2019 7:55 PM, Guido van Rossum wrote:
> Also, did anyone ask Davin directly to roll it back?

Simply put:  no.  There have been a number of reactionary comments in the
last 16 hours but no attempt to reach out to me directly during that time.


On Sun, Feb 3, 2019 at 8:12 PM Raymond Hettinger <
raymond.hettin...@gmail.com> wrote:
> It was only recently that you edited his name out of the list of
maintainers for multiprocessing
> even though that is what he's been working on for the last two years and
at the last two sprints.

I think it would be best to discuss Antoine's decision to take this
particular action without first consulting me, elsewhere and not part of
this thread.


As I said, I am happy to do the most constructive thing possible and I
sought the advice of those I highly respect first before doing as I have.


Davin


On Sun, Feb 3, 2019 at 9:12 PM Davin Potts <
python+python_...@discontinuity.net> wrote:

> I am attempting to do the right thing and am following the advice of other
> core devs in what I have done thus far.
>
> Borrowing heavily from what I've added to issue35813 just now:
>
> This work is the result of ~1.5 years of development effort, much of it
> accomplished at the last two core dev sprints.  The code behind it has been
> stable since September 2018 and tested as an independently installable
> package by multiple people.
>
> I was encouraged by Lukasz, Yury, and others to check in this code early,
> not waiting for tests and docs, in order to both solicit more feedback and
> provide for broader testing.  I understand that doing such a thing is not
> at all a novelty.  Thankfully it is doing that -- I hope that feedback
> remains constructive and supportive.
>
> There are some tests to be found in a branch (enh-tests-shmem) of
> github.com/applio/cpython which I think should become more comprehensive
> before inclusion.  Temporarily deferring and not including them as part of
> the first alpha should reduce the complexity of that release.
>
> Regarding the BSD license on the C code being adopted, my conversations
> with Brett and subsequently Van have not raised concerns, far from it --
> there is a process which is being followed to the letter.  If there are
> other reasons to object to the thoughtful adoption of code licensed like
> this one, that deserves a decoupled and larger discussion first.
>
>
> Davin
>
> On Sun, Feb 3, 2019 at 8:12 PM Raymond Hettinger <
> raymond.hettin...@gmail.com> wrote:
>
>>
>> > On Feb 3, 2019, at 5:40 PM, Terry Reedy  wrote:
>> >
>> > On 2/3/2019 7:55 PM, Guido van Rossum wrote:
>> >> Also, did anyone ask Davin directly to roll it back?
>> >
>> > Antoine posted on the issue, along with Robert O.  Robert reviewed and
>> make several suggestions.
>>
>> I think the PR sat in a stable state for many months, and it looks like
>> RO's review comments came *after* the commit.
>>
>> FWIW, with dataclasses we decided to get the PR committed early, long
>> before most of the tests and all of the docs. The principle was that bigger
>> changes needed to go in as early as possible in the release cycle so that
>> we could thoroughly exercise it (something that almost never happens while
>> something is in the PR stage).  It would be great if the same came happen
>> here.  IIRC, shared memory has long been the holy grail for
>> multiprocessing, helping to mitigate its principle disadvantage (the cost
>> of moving data between processes).  It's something we really want.
>>
>> But let's see what the 3.8 release manager has to say.
>>
>>
>> Raymond
>>
>>
>> ___
>> Python-Dev mailing list
>> Python-Dev@python.org
>> https://mail.python.org/mailman/listinfo/python-dev
>> Unsubscribe:
>> https://mail.python.org/mailman/options/python-dev/python%2Bpython_dev%40discontinuity.net
>>
>
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Davin Potts]

I am attempting to do the right thing and am following the advice of other
core devs in what I have done thus far.

Borrowing heavily from what I've added to issue35813 just now:

This work is the result of ~1.5 years of development effort, much of it
accomplished at the last two core dev sprints.  The code behind it has been
stable since September 2018 and tested as an independently installable
package by multiple people.

I was encouraged by Lukasz, Yury, and others to check in this code early,
not waiting for tests and docs, in order to both solicit more feedback and
provide for broader testing.  I understand that doing such a thing is not
at all a novelty.  Thankfully it is doing that -- I hope that feedback
remains constructive and supportive.

There are some tests to be found in a branch (enh-tests-shmem) of
github.com/applio/cpython which I think should become more comprehensive
before inclusion.  Temporarily deferring and not including them as part of
the first alpha should reduce the complexity of that release.

Regarding the BSD license on the C code being adopted, my conversations
with Brett and subsequently Van have not raised concerns, far from it --
there is a process which is being followed to the letter.  If there are
other reasons to object to the thoughtful adoption of code licensed like
this one, that deserves a decoupled and larger discussion first.


Davin

On Sun, Feb 3, 2019 at 8:12 PM Raymond Hettinger <
raymond.hettin...@gmail.com> wrote:

>
> > On Feb 3, 2019, at 5:40 PM, Terry Reedy  wrote:
> >
> > On 2/3/2019 7:55 PM, Guido van Rossum wrote:
> >> Also, did anyone ask Davin directly to roll it back?
> >
> > Antoine posted on the issue, along with Robert O.  Robert reviewed and
> make several suggestions.
>
> I think the PR sat in a stable state for many months, and it looks like
> RO's review comments came *after* the commit.
>
> FWIW, with dataclasses we decided to get the PR committed early, long
> before most of the tests and all of the docs. The principle was that bigger
> changes needed to go in as early as possible in the release cycle so that
> we could thoroughly exercise it (something that almost never happens while
> something is in the PR stage).  It would be great if the same came happen
> here.  IIRC, shared memory has long been the holy grail for
> multiprocessing, helping to mitigate its principle disadvantage (the cost
> of moving data between processes).  It's something we really want.
>
> But let's see what the 3.8 release manager has to say.
>
>
> Raymond
>
>
> ___
> Python-Dev mailing list
> Python-Dev@python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/python%2Bpython_dev%40discontinuity.net
>
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Raymond Hettinger]

> On Feb 3, 2019, at 5:40 PM, Terry Reedy  wrote:
> 
> On 2/3/2019 7:55 PM, Guido van Rossum wrote:
>> Also, did anyone ask Davin directly to roll it back?
> 
> Antoine posted on the issue, along with Robert O.  Robert reviewed and make 
> several suggestions.

I think the PR sat in a stable state for many months, and it looks like RO's 
review comments came *after* the commit.  

FWIW, with dataclasses we decided to get the PR committed early, long before 
most of the tests and all of the docs. The principle was that bigger changes 
needed to go in as early as possible in the release cycle so that we could 
thoroughly exercise it (something that almost never happens while something is 
in the PR stage).  It would be great if the same came happen here.  IIRC, 
shared memory has long been the holy grail for multiprocessing, helping to 
mitigate its principle disadvantage (the cost of moving data between 
processes).  It's something we really want.

But let's see what the 3.8 release manager has to say.


Raymond


___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Raymond Hettinger]

> On Feb 3, 2019, at 1:03 PM, Antoine Pitrou  wrote:
> 
> I'd like to ask for the reversion of the changes done in
> https://github.com/python/cpython/pull/11664

Please work *with* Davin on this one.

It was only recently that you edited his name out of the list of maintainers 
for multiprocessing even though that is what he's been working on for the last 
two years and at the last two sprints.  I'd like to see more team work here 
rather than applying social pressures via python-dev (which is a *very* public 
list). 


Raymond

___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Terry Reedy]

On 2/3/2019 7:55 PM, Guido van Rossum wrote:

Also, did anyone ask Davin directly to roll it back?


Antoine posted on the issue, along with Robert O.  Robert reviewed and 
make several suggestions.


--
Terry Jan Reedy

___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Guido van Rossum]

Also, did anyone ask Davin directly to roll it back?

On Sun, Feb 3, 2019 at 4:49 PM Guido van Rossum  wrote:

> I think this is now up to the 3.8 release manager.
>
> On Sun, Feb 3, 2019 at 4:34 PM Terry Reedy  wrote:
>
>> On 2/3/2019 4:03 PM, Antoine Pitrou wrote:
>> >
>> > Hello,
>> >
>> > I'd like to ask for the reversion of the changes done in
>> > https://github.com/python/cpython/pull/11664
>> >
>> > The reason is simple: [over 1000 lines not reviewed, no tests, no docs]
>>
>> Aside from the technical reasons Antoine gave, which I agree with, I
>> think the merge was legally questionable, as a non-contributor is listed
>> as a copyright holder.  Message 334805.
>> https://bugs.python.org/issue35813
>>
>> --
>> Terry Jan Reedy
>>
>> ___
>> Python-Dev mailing list
>> Python-Dev@python.org
>> https://mail.python.org/mailman/listinfo/python-dev
>> Unsubscribe:
>> https://mail.python.org/mailman/options/python-dev/guido%40python.org
>>
>
>
> --
> --Guido van Rossum (python.org/~guido)
>


-- 
--Guido van Rossum (python.org/~guido)
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Guido van Rossum]

I think this is now up to the 3.8 release manager.

On Sun, Feb 3, 2019 at 4:34 PM Terry Reedy  wrote:

> On 2/3/2019 4:03 PM, Antoine Pitrou wrote:
> >
> > Hello,
> >
> > I'd like to ask for the reversion of the changes done in
> > https://github.com/python/cpython/pull/11664
> >
> > The reason is simple: [over 1000 lines not reviewed, no tests, no docs]
>
> Aside from the technical reasons Antoine gave, which I agree with, I
> think the merge was legally questionable, as a non-contributor is listed
> as a copyright holder.  Message 334805. https://bugs.python.org/issue35813
>
> --
> Terry Jan Reedy
>
> ___
> Python-Dev mailing list
> Python-Dev@python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/guido%40python.org
>


-- 
--Guido van Rossum (python.org/~guido)
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Terry Reedy]

On 2/3/2019 4:03 PM, Antoine Pitrou wrote:


Hello,

I'd like to ask for the reversion of the changes done in
https://github.com/python/cpython/pull/11664

The reason is simple: [over 1000 lines not reviewed, no tests, no docs]


Aside from the technical reasons Antoine gave, which I agree with, I 
think the merge was legally questionable, as a non-contributor is listed 
as a copyright holder.  Message 334805. https://bugs.python.org/issue35813


--
Terry Jan Reedy

___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Asking for reversion

[Barry Warsaw]

On Feb 3, 2019, at 13:03, Antoine Pitrou  wrote:
> 
> I'd like to ask for the reversion of the changes done in
> https://github.com/python/cpython/pull/11664
> 
> The reason is simple: the PR isn't complete, it lacks docs and tests.
> It also didn't pass any review (this was pointed by Ronald), even
> though it adds 1300 lines of code.  No programmer is perfect, so it's
> statistically likely that the PR is defective.

I concur.  I actually think CI shouldn’t even pass without sufficiently 
covering tests and docs (sans a “trivial” or other short circuiting label), but 
that might be unpopular.

-Barry



signature.asc
Description: Message signed with OpenPGP
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

[Python-Dev] Asking for reversion

[Antoine Pitrou]

Hello,

I'd like to ask for the reversion of the changes done in
https://github.com/python/cpython/pull/11664

The reason is simple: the PR isn't complete, it lacks docs and tests.
It also didn't pass any review (this was pointed by Ronald), even
though it adds 1300 lines of code.  No programmer is perfect, so it's
statistically likely that the PR is defective.

With git, forks and branches, we definitely don't need to commit
unfinished PRs to the main repo.  It's perfectly fine to maintain
some non-trivial piece of work in a separate fork.  People do it on a
regular basis (for example I have currently two such
long-lived branches: one for PEP 556 and one for PEP 574).


Also, this is not the first time this happened.  Another
multiprocessing PR was merged some years ago without any docs or tests:
https://bugs.python.org/issue28053

Today that work /still/ lacks docs or tests, and there is a suspicion
that it doesn't work as intended (see issue comments).  It's probably
too late to revert it, but it's definitely a slippery slope.

Regards

Antoine.


___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com