[issue27137] Python implementation of `functools.partial` is not a class

[Emanuel Barry]

Emanuel Barry added the comment:

Thank you Nick! I just opened #28062 to fix the repr inconsistency between 
functools.partial and any subclass :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Roundup Robot]

Roundup Robot added the comment:

New changeset cdc91b6ae3b2 by Nick Coghlan in branch 'default':
Issue #27137: align Python & C implementations of functools.partial
https://hg.python.org/cpython/rev/cdc91b6ae3b2

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Nick Coghlan]

Nick Coghlan added the comment:

Thanks folks - for 3.6, the pure Python and accelerated C implementations of 
functools.partial are now both multiprocessing, subclassing, and REPL friendly 
:)

--
resolution:  -> fixed
stage: commit review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Emanuel Barry]

Emanuel Barry added the comment:

The patch LGTM and applies fine. Looks like there's no need to wait for beta 2 
after all; thanks Serhiy!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Here is a patch that makes C implementation repr matching Python implementation 
repr. It should be applied to all Python versions (the code is new).

--
Added file: 
http://bugs.python.org/file44483/functools_partial_recursive_repr.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Brett Cannon]

Brett Cannon added the comment:

Ned will make the call about committing broken tests decorated with the 
expected failure decorator, but if you view this as a bug then this can go into 
b2 w/ passing tests instead.

--
nosy: +ned.deily

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Emanuel Barry]

Emanuel Barry added the comment:

Can this be merged even with the two failing tests? I have little to no time 
available to fix it properly before the feature freeze. We can skip/silence 
those tests for a bit; I'll open a new issue and fix it in time for beta 2.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Emanuel Barry]

Emanuel Barry added the comment:

Thank you Serhiy for the comments! Here's a new patch. I'm fine with the 
recursive repr tests failing for now; it's something I believe we can fix 
during the beta phase if we don't have time before (fix as in modify the C 
implementation to match the Python version instead of the other way around).

--
Added file: http://bugs.python.org/file44450/functools_partial_4.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I think we can change C implementation of __repr__ to match straightforward 
Python implementation.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Emanuel Barry]

Emanuel Barry added the comment:

Hi Nick, thank you for letting me know! I started trying to fix this, however I 
found it very hard to fix the recursive repr issue. I've whipped up an 
incomplete (but yet working) patch that fixes all but the recursive repr issue. 
Only those two tests fail (once for functools.partial and once for the 
subclass). I have to go for now, but feel free to play with this patch and see 
if you can fix it. The problem is that the way it's handled is inconsistent 
within the C implementation, and is incompatible with reprlib.recursive_repr. I 
might try my hand at it later today or tomorrow.

In the meantime, I think maybe the C implementation cares too much about the 
special cases for that.

--
Added file: 
http://bugs.python.org/file44436/functools_partial_3_1_incomplete.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Nick Coghlan]

Nick Coghlan added the comment:

Attempting to apply this, I found that the pure Python fallback fails the new 
recursive repr tests added in https://bugs.python.org/issue25455

The recursive pickling test is also failing, but I suspect that may be due to 
an error in my attempt to apply parts of the patch manually after they failed 
to apply due to the #25455 changes.

Emanuel, would you be able to take another pass at this and resolve those 
inconsistencies? Otherwise I should be able to get to it on the weekend (and 
hence still in time for beta 1)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Nick Coghlan]

Nick Coghlan added the comment:

Thanks for the nudge, and sorry for getting distracted! I won't be able to get 
to this today, but I'll definitely handle it in time for the deadline :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Emanuel Barry]

Emanuel Barry added the comment:

We're one week from the feature freeze, seems like a good time to merge this :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Nick Coghlan]

Nick Coghlan added the comment:

Emanuel's latest patch looks good to me, but after seeing yet another behaviour 
discrepancy between the two versions(static method vs instance method 
behaviour), I've proposed disallowing function/class mismatches as an explicit 
policy clarification in PEP 399: 
https://mail.python.org/pipermail/python-dev/2016-July/145556.html

Since this change can land any time before the first 3.6 beta in September, I'm 
going to wait and see want kind of responses we get to that thread before 
merging it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Nick Coghlan]

Nick Coghlan added the comment:

I should be able to take a look at the updated patch this coming weekend.

--
stage: patch review -> commit review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Emanuel Barry]

Emanuel Barry added the comment:

Is there anything still preventing this from being merged? I think the ability 
to consistently get the same behaviour, with or without _functools available, 
outweighs the performance and simplicity loss (and as far as performance loss 
goes, it appears that _functools is always available as far as CPython is 
concerned, which makes it a non-issue).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Emanuel Barry]

Emanuel Barry added the comment:

New patch, now almost all the tests that are used for the C version are also 
used for the Python version (the only exception is the one checking that trying 
to change/delete attributes would raise an error). Some tests needed a bit of 
tweaking to work with the implementation details of both versions, but now all 
of the tests pass (even the pickle one, which I found how to make it pass while 
working on #27220).

--
Added file: http://bugs.python.org/file43212/functools_partial_3.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Nick Coghlan]

Nick Coghlan added the comment:

As Raymond notes, the main downside here is in terms of code complexity. 
However, the concrete gain is that APIs that rely on callable pickling, such as 
concurrent.futures with a ProcessPoolExecutor, would be consistently compatible 
with functools.partial:

>>> from concurrent.futures import ProcessPoolExecutor
>>> from functools import partial
>>> with ProcessPoolExecutor() as pool:
... pool.submit(print, "hello")
... pool.submit(partial(print, "hello"))
... 


hello
hello

At the moment, such code will fail if _functools is unavailable, since closures 
don't support pickling (unpickling functions involves looking them up by name, 
which isn't possible with closures)

The other main benefit is retaining the custom __repr__ when falling back to 
the Python implementation:

>>> partial(print, "hello")
functools.partial(, 'hello')

At the moment, the closure-based version instead gives:

>>> partial(print, "hello")
.newfunc at 0x7f4fd6e0aea0>

Preserving those two capabilities seems sufficiently worthwhile to me to 
justify the extra code complexity and the greater speed penalty when the 
accelerator module isn't available (I'm assuming that in runtimes using a JIT 
compiler the speed difference should be negligible in practice)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[Emanuel Barry]

Emanuel Barry added the comment:

New patch after Serhiy's comments. I kept the __dict__ in pickling to properly 
pickle attributes that aren't defined by 'partial' itself; but if 'func', 
'args' or 'keywords' are present and don't match the provided arguments, an 
error will be raised, as it should not be partial's responsibility to handle 
this; this is for the caller to do.

--
Added file: http://bugs.python.org/file43059/functools_partial_2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue27137] Python implementation of `functools.partial` is not a class

[ppperry]

Changes by ppperry :


--
title: functools.partial: Inconsistency between Python and C implementations -> 
Python implementation of `functools.partial` is not a class

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com