classification
Title: Add functools.partialclass
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.8
process
Status: closed Resolution: later
Dependencies: Superseder:
Assigned To: Nosy List: NeilGirdhar, eric.smith, ncoghlan, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2018-05-03 18:31 by NeilGirdhar, last changed 2018-05-10 15:49 by ncoghlan. This issue is now closed.

Files
File name Uploaded Description Edit
partialclass.diff NeilGirdhar, 2018-05-03 18:31 review
partialclass2.diff NeilGirdhar, 2018-05-03 18:33 review
partialclass3.diff NeilGirdhar, 2018-05-03 22:18 Made every subject have its own paragraph. review
Pull Requests
URL Status Linked Edit
PR 6699 closed python-dev, 2018-05-03 23:10
Messages (14)
msg316126 - (view) Author: Neil Girdhar (NeilGirdhar) * Date: 2018-05-03 18:31
functools.partial is almost good enough for specifying some of the parameters of an object's initializer, but the partial object doesn't respond properly to issubclass.  Adding functools.partialclass is similar to the addition of partialmethod in 3.4.
msg316128 - (view) Author: Neil Girdhar (NeilGirdhar) * Date: 2018-05-03 18:40
I edited some of the documentation as well to use the technical terms "partial function application", "partial method application", and "partial class application".  This emphasizes the parallel structure and reduces confusion with "partial functions" that mean something else in other languages.

I also removed a phrase that used the term "freezes" since in Python "frozen" is also used in frozenset to mean frozen for good whereas the keyword arguments in a partial function application are overridable.
msg316138 - (view) Author: Neil Girdhar (NeilGirdhar) * Date: 2018-05-03 22:00
Added functools experts.

Links to relevant stackoverflow questions:

https://stackoverflow.com/questions/38911146/python-equivalent-of-functools-partial-for-a-class-constructor

https://stackoverflow.com/questions/50143864/is-there-a-nice-way-to-partially-bind-class-parameters-in-python/50143893
msg316139 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-05-03 22:16
This has to be a 3.8 feature.

It would be best if you could convert this to a github pull request, and use blurb to generate the news entry. But those aren't strictly necessary, someone else can do the mechanics of that. I'm willing to do it if you'd like, and at the appropriate time.
msg316140 - (view) Author: Neil Girdhar (NeilGirdhar) * Date: 2018-05-03 22:19
I figured it would have to be 3.8, but it looks like Doc/whatsnew/3.8.rst has not been created?  Do I create that?
msg316141 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-05-03 22:21
You'll need to use blurb:

https://pypi.org/project/blurb/
https://devguide.python.org/committing/#what-s-new-and-news-entries

Well, technically it's possible to not use blurb and do it manually, but you'll want to use it.
msg316144 - (view) Author: Neil Girdhar (NeilGirdhar) * Date: 2018-05-03 23:18
Done: https://github.com/python/cpython/pull/6699
msg316161 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-04 09:52
I'm not sure that this should be in the stdlib. The three-line function can be enough for your simple case, and it is too simple for including it in the stdlib. But for general stdlib quality solution it lacks many details.

1. It doesn't work with classes that implement the constructor as __new__, but not __init__. It may need of using metaclasses with overridden __call__. But how then handle classes with custom metaclasses?

2. inspect.signature() doesn't give the correct signature as for partial(). This requires changing the inspect module.

3. pickling and copying of instances are broken in many cases. Pickling a class is always broken. I afraid that this can't be solved without significant reworking of the pickle and the copy modules.

4. It adds instance attributes __dict__ and __weakref__ even if they were disabled by using __slots__. This increases the size of instances and breaks some properties.

5. repr() of the class doesn't show that it is a partialclass (unlike to the result of partial()).

6. Many alternate constructors and copying methods are broken. For example the copy() method of partialclass(defaultdict, list) in your example. There is no general solution of this, it should be solved individually for every class.

If there is a need of this feature in the stdlib, it needs changing many parts of the stdlib. And the result will still need an additional work for every concrete class.

Also note that the term "partial class" has different well known meaning in some other programming languages: https://en.wikipedia.org/wiki/Class_(computer_programming)#Partial .
msg316167 - (view) Author: Neil Girdhar (NeilGirdhar) * Date: 2018-05-04 11:40
I'm not sure that this should be in the stdlib. The three-line function can be enough for your simple case, and it is too simple for including it in the stdlib. But for general stdlib quality solution it lacks many details.

1. It doesn't work with classes that implement the constructor as __new__, but not __init__. It may need of using metaclasses with overridden __call__. But how then handle classes with custom metaclasses?

Can you illustrate how you would do it for these kinds of classes?

Anyway, I think using __new__ on user classes is extremely rare.

2. inspect.signature() doesn't give the correct signature as for partial(). This requires changing the inspect module.

Good point.  I can look into this.

3. pickling and copying of instances are broken in many cases. Pickling a class is always broken. I afraid that this can't be solved without significant reworking of the pickle and the copy modules.

Okay, interesting, but this doesn't seem relevant to partialclass.

4. It adds instance attributes __dict__ and __weakref__ even if they were disabled by using __slots__. This increases the size of instances and breaks some properties.

Can we just copy the parent class' __slots__ to the partialclass return value?

5. repr() of the class doesn't show that it is a partialclass (unlike to the result of partial()).

I will fix this.

6. Many alternate constructors and copying methods are broken. For example the copy() method of partialclass(defaultdict, list) in your example. There is no general solution of this, it should be solved individually for every class.

Ultimately, if pickling works, then copying should work too.  The usual way I do it is implementing __getnewargs__, etc.   This should work fine?

> If there is a need of this feature in the stdlib, it needs changing many parts of the stdlib. And the result will still need an additional work for every concrete class.

Fair enough.

> Also note that the term "partial class" has different well known meaning in some other programming languages: https://en.wikipedia.org/wiki/Class_(computer_programming)#Partial .

Interesting.  Similarly "partial function" means something else.  That's why I changed the documentation to use the terms: "partial class application", "partial function application", and "partial method application".  All of these are "partial applications": https://en.wikipedia.org/wiki/Partial_application .

Should we discuss this on github?
msg316205 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-05 11:22
> I'm not sure that this should be in the stdlib. The three-line function can be enough for your simple case, and it is too simple for including it in the stdlib. But for general stdlib quality solution it lacks many details.
>
> 1. It doesn't work with classes that implement the constructor as __new__, but not __init__. It may need of using metaclasses with overridden __call__. But how then handle classes with custom metaclasses?
>
> Can you illustrate how you would do it for these kinds of classes?

I wouldn't try to implement the generic partialclass() at first place. 
But if I should, the possible code:

     class PartialMeta(type(cls)):
         __call__ = partialmethod(type(cls).__call__, *args, **kwargs)
     class PartialClass(cls, metaclass=PartialMeta):
         pass
     return PartialClass

Not sure it works.

Another solution -- add a wrapped __new__, but __new__ and __init__ 
should be added only when they where defined in the original class or 
its parenthesis. There may be caveats with inheriting from some builtin 
classes.

> Anyway, I think using __new__ on user classes is extremely rare.

This is not good reason of not supporting them. The library function 
should support all corner cases.

> 2. inspect.signature() doesn't give the correct signature as for partial(). This requires changing the inspect module.
>
> Good point.  I can look into this.
>
> 3. pickling and copying of instances are broken in many cases. Pickling a class is always broken. I afraid that this can't be solved without significant reworking of the pickle and the copy modules.
>
> Okay, interesting, but this doesn't seem relevant to partialclass.

If you generate a class, it should behave correctly. Otherwise you could 
just use partial().

> 4. It adds instance attributes __dict__ and __weakref__ even if they were disabled by using __slots__. This increases the size of instances and breaks some properties.
>
> Can we just copy the parent class' __slots__ to the partialclass return value?

This is not so easy. You should add an empty __slots__ if the original 
class doesn't have __dict__ in instances. And testing that may be 
non-trivial task.

> 6. Many alternate constructors and copying methods are broken. For example the copy() method of partialclass(defaultdict, list) in your example. There is no general solution of this, it should be solved individually for every class.
>
> Ultimately, if pickling works, then copying should work too.  The usual way I do it is implementing __getnewargs__, etc.   This should work fine?

I meant the defaultdict.copy() method, not the copy module,

> Should we discuss this on github?

The bug tracker is the place for the design discussion. GitHub is a 
place for discussing implementation details.

It looks to me that this should first be discussed on Python-ideas. You 
should convince core developers that it is worth to add this feature, 
and provide a plan of solving all implementation problems mentioned above.
msg316215 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-05-05 15:08
Note that Neil did start with a python-ideas discussion, and was directed over here, since the idea seemed simple enough, and worth pursuing.

As Serhiy notes though, there are many more subtleties to be addressed here than I first thought.

That said, as long as __init__, __new__ and __slots__ are handled appropriately in the partial subclass, I think custom metaclasses should largely take care of themselves, so it would be better to avoid the compatibility implications of injecting an implicit metaclass.

The slots case should be adequately handled by doing:

    if hasattr(cls, "__slots__"):
        __slots__ = ()

The __new__ case may have some quirks due to the fact it's technically implemented as an implicitly static method, but I *think* that can be covered by defining it as:

    if cls.__new__ is not object.__new__:
        __new__ = partialmethod(cls.__new__, *args, **kwds)

and relying on the native class machinery to include the same kind of fixup that it applies for any other __new__ method implementation.

__init__ will need a similar "has it been overridden?" check to the one in __new__ (comparing the unbound methods via "cls.__init__ is not object.__init__").


Some of the other issues that Serhiy mentions are real problems with the approach (like pickle compatibility, alternate constructor support, etc), but I think those can simply be noted in the documentation, with the following double-subclassing recipe noted as a suggested way of handling them:

    class MySubclass(partialclass(BaseClass, *args, **kwds)):
        ...
        # MySubclass supports pickle as it's reachable by name
        # Custom constructors can be overloaded as needed here

(Note: if you're not seeing https://github.com/python/cpython/blob/master/Doc/whatsnew/3.8.rst locally, check that you're accidentally working on the 3.7 branch, instead of the master branch)
msg316284 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-08 11:06
I'm -1 on this change. It looks as an uncommon case, and the correct general implementation is hard (it may be even more hard that it looked to me now). The stdlib should't provide an incomplete implementation which can be replaced with just three lines of code in a concrete case if the user doesn't care about subtle details.
msg316324 - (view) Author: Neil Girdhar (NeilGirdhar) * Date: 2018-05-09 14:39
It seems like Python doesn't do very well with dynamically-generated classes.

For that reason, I'm losing interest on this feature.

Is there any interest in merging the documentation changes here: https://bugs.python.org/review/33419/diff/20050/Doc/library/functools.rst ?

I think we should use the terminology "partial function application" (see https://en.wikipedia.org/wiki/Partial_application) rather than "partial function".

Also, the existing paragraphs change subject from talking about the library function (e.g., partial) to the function returned by the library function, which is confusing.  I thought we should break them up.

Finally, the term "freezing" an interface is a bit weird since Python uses freezing in other another context to mean immutable (frozenset), and the interface returned by partial is overridable.

Also, I realize I didn't wait very long, but I think it would be best in the future to register one's "–1" on pyhton-ideas before someone embarks on implementing something everyone else approved.  This also happened with PEP 448 too after I'd worked on it for weeks, and it was very frustrating.

Anyway, feel free to close.  If this keeps coming up over the next few years, someone else can work from this patch.
msg316369 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-05-10 15:49
Marking as closed/later to indicate that we're not going to pursue this in the near term, but it's not out of the question that we might accept a future proposal along these lines.
History
Date User Action Args
2018-05-10 15:49:58ncoghlansetstatus: open -> closed
resolution: later
messages: + msg316369

stage: patch review -> resolved
2018-05-09 14:39:41NeilGirdharsetmessages: + msg316324
2018-05-08 11:06:27serhiy.storchakasetmessages: + msg316284
2018-05-05 15:08:08ncoghlansetmessages: + msg316215
2018-05-05 11:22:06serhiy.storchakasetmessages: + msg316205
2018-05-04 11:40:32NeilGirdharsetmessages: + msg316167
2018-05-04 09:52:28serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg316161
2018-05-03 23:18:11NeilGirdharsetmessages: + msg316144
2018-05-03 23:10:41python-devsetstage: patch review
pull_requests: + pull_request6391
2018-05-03 22:21:36eric.smithsetmessages: + msg316141
2018-05-03 22:19:25NeilGirdharsetmessages: + msg316140
2018-05-03 22:18:16NeilGirdharsetfiles: + partialclass3.diff
2018-05-03 22:16:28eric.smithsetnosy: + eric.smith

messages: + msg316139
versions: + Python 3.8, - Python 3.7
2018-05-03 22:00:08NeilGirdharsetnosy: + rhettinger, ncoghlan
messages: + msg316138
2018-05-03 18:40:32NeilGirdharsetmessages: + msg316128
2018-05-03 18:33:08NeilGirdharsetfiles: + partialclass2.diff
2018-05-03 18:32:02NeilGirdharsettype: enhancement
components: + Library (Lib)
2018-05-03 18:31:32NeilGirdharcreate