Python implementation of `functools.partial` is not a class

[Vgr255]

BPO	27137
Nosy	@brettcannon, @rhettinger, @ncoghlan, @vstinner, @ned-deily, @serhiy-storchaka, @zhangyangyu, @Vgr255
Files	functools_partial_1.patch functools_partial_2.patch functools_partial_3.patch functools_partial_3_1_incomplete.patch functools_partial_4.patch functools_partial_recursive_repr.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/ncoghlan'
closed_at = <Date 2016-09-10.10:02:04.039>
created_at = <Date 2016-05-27.16:39:18.782>
labels = ['type-bug', 'library']
title = 'Python implementation of `functools.partial` is not a class'
updated_at = <Date 2016-09-10.12:13:36.524>
user = 'https://github.com/Vgr255'

bugs.python.org fields:

activity = <Date 2016-09-10.12:13:36.524>
actor = 'abarry'
assignee = 'ncoghlan'
closed = True
closed_date = <Date 2016-09-10.10:02:04.039>
closer = 'ncoghlan'
components = ['Library (Lib)']
creation = <Date 2016-05-27.16:39:18.782>
creator = 'abarry'
dependencies = []
files = ['43035', '43059', '43212', '44436', '44450', '44483']
hgrepos = []
issue_num = 27137
keywords = ['patch']
message_count = 29.0
messages = ['266503', '266530', '266534', '266539', '266540', '266545', '266570', '266587', '266588', '266599', '266700', '267003', '267334', '269899', '269917', '270043', '274476', '274522', '274768', '274809', '274827', '274890', '275232', '275240', '275251', '275305', '275609', '275610', '275629']
nosy_count = 9.0
nosy_names = ['brett.cannon', 'rhettinger', 'ncoghlan', 'vstinner', 'ned.deily', 'python-dev', 'serhiy.storchaka', 'xiang.zhang', 'abarry']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue27137'
versions = ['Python 3.6']

[Vgr255]

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?

[Vgr255]

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?

[rhettinger]

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

[serhiy-storchaka]

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 (bpo-24164) and for supporting pickling of operator.methodcaller() objects with keyword arguments (bpo-22955).

[rhettinger]

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.

[serhiy-storchaka]

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.

[Vgr255]

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

[vstinner]

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

[serhiy-storchaka]

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

[Vgr255]

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?