| 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) *  |
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) * |
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) * |
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) * |
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: Alyssa Coghlan (ncoghlan) * |
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) * |
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: Alyssa Coghlan (ncoghlan) * |
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. |
|
|