GeezerVison is showing github.com — Add __get__ to the partial object · Issue #121027 · python/cpython
Add __get__ to the partial object #121027DescriptionFeature or enhancement
In #119827 (comment), @rhettinger proposed to add the __get__ method to the partial object in functools. This is a breaking change, although the impact may be much lesser than of adding __get__ to builtin functions. But we should follow the common procedure for such changes: first add __get__ that emits FutureWarning with suggestion to wrap partial into staticmethod and return the partial object unchanged, then change the behavior few releases later.
Linked PRs
Activityncoghlan commented on Jun 26, 2024 The "Does practicality beat purity?" question also applies here.
Yes, if we'd had the placeholder functionality all along we might never have added partialmethod, but given that we did add it, what are we gaining by adding partial.__get__ that justifies the migration cost of having to wrap instances in staticmethod to get the old behaviour back when it was actually desired? serhiy-storchaka commented on Jun 26, 2024 I do not know. I have not found any issue which could be solved by adding __get__ to partial. The migration cost could be not high, but the benefit is unclear to me. It may simplify the partialmethod implementation in long run, but I did not check this. serhiy-storchaka commented on Jun 26, 2024 For clarification, I do not oppose this feature. But there is a procedure for making breaking changes. It should not bit buried as a side effect of adding other feature.
This can be worked around with significant core developer support if we decide the benefit of immediate change outweighs the risk. Or if we reclassify it as a bugfix. rhettinger commented on Jun 26, 2024
what are we gaining by adding partial.get
We are avoiding a problem that formerly didn't present itself. Partial is almost entirely understood as being an equivalent to a lambda or def. Before `Placeholder' was added, no one would encounter this. Now, it is a certitude.
Without adding __get__, one user after another will have to discover the hard way that these two are not equivalent:
set_alive2 = lambda self: self.set_state(True)
set_alive3 = partial(set_state, Placeholder, True)
When they learn about the non-equivalence, it will break their mental model of partial() as it did for me.
Next, they will have to discover a workaround with partialmethod() which is mostly unused, mostly unknown, and really shouldn't be necessary anymore.
I don't think it should be acceptable to leave partial() in a non-harmonious state just because partialmethod() exists. The mental model of equivalence with lambda and def is too important. Reacted by Nice Zombies, Alyssa Coghlan, dgpb, Max Kühn, jfs and Jacob Chapman rhettinger commented on Jun 26, 2024 Also, I don't think there is much of a downside. The docs for partial() don't promise to not act like a regular function in a class. It is certainly isn't an advertised feature of partial() that "I am just like an equivalent def or lambda but can't be used as a method."
There might be some minor disruption but it leads to a better designed more clear outcome than being trapped into a surprising behavior that was never intended.
If __get__ is not added, I'm thinking of withdrawing my support for Placeholder because it would make this wart prominent and it would limit natural uses of the new feature. serhiy-storchaka commented on Jun 26, 2024 As an option, we can add __get__ that emits a FutureWarning in 3.13 and change the behavior in 3.14. This will not delay introducion of Placeholder.
It may be late for a beta stage, but the premise is that it will not affect much user code. If this is true, adding it in 3.13 is safe. If it is not true, it is still better than breaking that code without a warning.
Surprisingly, it only breaks few recently added tests in the CPython test suite, and these tests were added specially to test how the code handles such weird corner case. They do not establish the correct behavior, they test that the current behavior will not be changed unintentionally.
cc @Yhg1s as the RM rhettinger commented on Jun 26, 2024 +1 for adding a FutureWarning to 3.13 if it is still possible. I expect that people with encounter it rarely and that the workaround is to just wrap partial with staticmethod making the behavior explicit.
It would also be reasonable to just put a note in the docs. This wasn't a behavior previously promised in the docs and isn't something that people would typically encounter. I've taught partial to thousands of people and only noticed the absence of __get__ when the new Placeholder option became available.
get the old behaviour back when it was actually desired?
As near as I can tell, this is almost never desired. And as a code reviewer if someone wanted this behavior, it should be made explicit because almost no one knows about it and it is surprising.
PeterJCLaw commented on Jul 4, 2024
In #119827 (comment), @rhettinger proposed to add the __get__ method to the partial object in functools. This is a breaking change, although the impact may be much lesser than of adding __get__ to builtin functions. But we should follow the common procedure for such changes: first add __get__ that emits FutureWarning with suggestion to wrap partial into staticmethod and return the partial object unchanged, then change the behavior few releases later.
I'm not sure whether your comment "This is a breaking change" is intended to refer to the future change to make the __get__ method return something other than self, or to the change as currently implemented, though I'm guessing the former. Either way, I wanted to highlight that the change as implemented may already be breaking for some use-cases. In particular tools which rely on introspection of code objects may be caught by this, as Jedi has been (it renders function signatures for method descriptors differently than other functions or class instances). Jedi can almost certainly cope with this, so I'm not arguing either for or against this change -- just wanted to share the impact that this has already had in case there might be other consequences of functools.partial being detected to be a descriptor. serhiy-storchaka commented on Jul 5, 2024 It referred to the future change, but, unfortunately, adding even trivial __get__() that returns self is a breaking change. It required changing the inspect.signature() code, and may also affect the third-party code. There is no good solution for this.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|