functools.partial is a class in C, but the Python implementation is a function. This doesn't matter for most use cases where you only want the end result of the call to partial.

A simple line in the REPL tells me enough (or so I thought) that I wouldn't need to check the implementation:

>>> functools.partial
<class 'functools.partial'>

Oh, it's a class, which means I can subclass it to add a custom repr for my needs.

Unsurprisingly, it works. It may not be the best idea to subclass something that is meant to be final, but I'm simply overriding a single method, what could possibly go wrong? Besides one of the implementations not actually being a class.

I'm suggesting to make the Python implementation also a class, for consistency and making sure that both the C and Python implementation match, in case someone else wants to do that too.

The documentation ( https://docs.python.org/3/library/functools.html#functools.partial ) doesn't state that the Python and C implementations differ, but IMO this isn't a documentation bug.

I haven't written a patch yet, will probably be done by tomorrow.

Note: I haven't actually encountered this issue, but I suspect that it might arise if someone doesn't have access to _functools for whatever reason. And IMO, Python and C implementations of a feature should be fully equivalent (modulo implementation details à la OrderedDict.__root).

Thoughts?

Attached patch turns the Python implementation of functools.partial into a class. The implementation is as close to the C version as possible, except __repr__ where I went for a different, more Pythonic approach (output is identical).

I haven't yet added tests for this, and I had to fix something (because some errors occur if _functools is not present and that's the only way I know to make the tests run against the Python implementation). One test fails with the patch (and sys.modules["_functools"] = None):

...........................................................E....................
....................................................................
======================================================================
ERROR: test_pickle (test.test_functools.TestPartialC)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "E:\GitHub\cpython\lib\test\test_functools.py", line 224, in test_pickle
    f_copy = pickle.loads(pickle.dumps(f, proto))
_pickle.PicklingError: Can't pickle <class 'functools.partial'>: it's not the sa
me object as functools.partial

----------------------------------------------------------------------
Ran 148 tests in 0.216s

FAILED (errors=1)

I'll try to see what kind of tests I can add to this to reliably test both 'partial' implementations. I'll also see what I can do about that one failing test. Oddly enough, there seems to be a test for subclassing '_functools.partial', which I might extend.

Raymond, what do you think?

-1 I prefer to keep the current clean, easy-to-understand, easy-to-get-right code which complies with the documented API.  I see no real benefit in twisting the code around to match non-guaranteed implementation details.

One problem with current Python implementation is that it doesn't support pickling. The ability of pickling functools.partial() is used in the pickle module for supporting __newobj_ex__ with pickle protocols 2 and 3 (issue24164) and for supporting pickling of operator.methodcaller() objects with keyword arguments (issue22955).

This kind of feature creep has a cost in tern complexity, maintenance, risk of bugs, loss of a clean example for others learn from, and in slower code.

If the Python implementation is just a rough example, and the functools module always require the _functools module, exact behavior of the Python implementation is not very important, as well as its performance. If the functools module can be used without C implementation, Python implementation is just not completely compatible.

Serhiy, it seems as though _functools is always required for functools to work - heck, tests start to fail all over the place if it isn't available, because functools.reduce doesn't exist.

Subclassing _functools.partial is already tested for, so I wouldn't qualify it as an implementation detail myself. Moreover, I feel that we should try to (somewhat) keep both implementations identical - or close enough.

It seems that _pickle checks for _functools.partial, and that trying to pickle the pure Python version triggers the error in msg266530.

This probably doesn't matter for most (if not all) of cases where CPython is concerned. But I care about the ability for alternate implementations to work the same way.

Compromise: rename the current partial function to partial_func and keep it around ;-)

The PEP 399 requires that the Python implementation behaves like the C
accelerator, no? I didn't check the specific case of this issue.

PEP 399 requires that the C code must pass the test suite used for the pure Python code.

Functionally, both versions are equivalent, but one is a class and the other is not. From PEP 399:

"Technical details of the VM providing the accelerated code are allowed to differ as necessary, e.g., a class being a `type` when implemented in C."

It clearly states that it's an implementation detail. I've been beaten! However, the next paragraph also says this:

"Acting as a drop-in replacement also dictates that no public API be provided in accelerated code that does not exist in the pure Python code."

I think this contradicts itself, since IMO an object being a type is part of the public API (but obviously this thought isn't universal). Whatever happens with the pure Python implementation, shouldn't the PEP be clarified on that point?

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.

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"))
... 
<Future at 0x7f4fdb47ce48 state=running>
<Future at 0x7f4fd4f9cb00 state=pending>
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(<built-in function print>, 'hello')

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

>>> partial(print, "hello")
<function functools.partial.<locals>.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)

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).

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).

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

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.

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

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 :)

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)

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.

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

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).

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.

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.

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).

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

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

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

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