-
-
Notifications
You must be signed in to change notification settings - Fork 30.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
functools.partial: Allowing trailing Placeholders #128644
Comments
Personally, I like that a The relevant Zen of Python is "errors should never pass silently" and "in the face of ambiguity, refuse the temptation to guess." My recommendation is to defer this. Let users gain experience with If a compelling case for trailing placeholders does arise, we can easily add it later. However, if it was added now and turned out to be a net detriment, it would be difficult to undo (deprecation and removal now takes half a decade). Also, if the goal is convert all arguments to positional-only (something that I almost never need to do), the trailing placeholder approach is not an obvious way either to do it, either for a code writer or to a code reader. I would not want to see this in production code that I cared about. |
Also "There should be one-- and preferably only one --obvious way to do it." With a trailing
Speaking of which, I'd like to hear more about this use-case. If something is supposed to be positional-only, then generally that would be done at the function-level, not in |
I see it this way. From the perspective of the algorithm, it can be summed up as follows: "Placeholder indicates that argument will need to be filled on If trailing Furthermore, the implementation without restriction:
So all of the above is for free without additional effort. As opposed to restricting them, which is extra work - both on the implementation and mental model aspects. I did this initially as initial implementation was immature and this restriction made it work, which is now gone and the current one handles leading placeholders appropriately (as it should). To answer Zen challenges: I don't think any guessing is needed here as functionality is summed up in 1 statement and everything else is a natural consequence. This is not an error and I don't think it should be seen as such. If this is an error, then Same applies to 1 obvious way. This adds an extra way to do it as much as: def foo(a):
return a
def bar(a):
return foo(a) And to this specific case it applies even less as this actually has an effect. I think this case might be more on the side of "consenting adults". |
Postponing is an option, but given the case is reasonably clear, determining the appropriate path forward now has benefits of releasing better implementation, having closed case and eliminating the possibility of someone depending on handling |
In my estimation,
I've read, re-read, and understood everything you've said but still do not think trailing placeholders are a good idea. To my eyes, they look weird and unnecessary. AFAICT, no one needs this. If someone is programmatically adding a variable number of placeholders (why, I'm not sure), then is easy to explicitly strip the rightmost placeholders. If someone wants to force a positional-only signature (again not sure why), they should do so in a more explicit manner rather than indirectly with a partial/placeholder combination. The common and intended use case doesn't need this at all.
I don't like that definition. It feels contrived to justify trailing placeholders, and it expresses an implementation centric viewpoint rather than how users would actually think about placeholders (and how we should advertise them). The actual intended purpose of
Note the plural here. The proposal also allows
I will leave this open for a while in case other core-devs feel like this is something they want users to be doing. But unless strong support is expressed, we should pass on this one. |
It is kind of implementation centric, but that is not necessarily a bad thing.
def func(a=1, b=2, c=3):
return a + b + c
signature(func) # (a=1, b=2, c=3)
p = partial(func, Placeholder, Placeholder, Placeholder)
signature(p) # (a, b, c, /) Not sure if there are actual use-cases for this, but maybe others will find this useful. E.g. ensuring that parameters such as The last time I advocated for not restricting something ended up realising that restriction was a good idea. Although this case feels a bit different, I don't want to make the same mistake. I am undecided now, need more time to think about it. |
By the way, this is another possibility for keyword Keyword Placeholder could be extended for this. I.e. Keyword placeholder would hold a place which needs to be filled on I have pretty much concluded that This is much simpler and is realistic (given this is a desirable feature). |
The path of converting keyword arguments to positional, reordering arguments, etc is complex and While this one is simple and would be pretty much final. The complete concept would read like: "Placeholder indicates that argument will need to be filled on |
This would allow things such as: def update_dicts(d, **kwds):
d.update(kwds)
names = dict()
update_names_compulsory_key_a = partial(update_dict, names, a=Placeholder) This could be a reasonable final step for I think it is worthwhile to keep this open as IMO in time (with increasing conviction that possible alternative extensions are better covered by other concepts/tools) this has a non-negligible chance to turn out to be a good way forward for |
ISTM that the core problem we set out to solve has been solved and it would be best to just stop here. It now feels like we're now entering the territory of imagination and invented problems. The risk is of damaging something that is currently clean. The entire premise of approving the It is my strong recommendation that you enjoy and use what has already been landed. In art school, they say that the hardest thing to teach is knowing when to stop painting. For now, I'm going to mark this as closed. Now other commenter has stepped forward in support of trailing placeholders and it is my belief that it is would be detrimental to users. If a compelling use case arises, we can re-open this and discuss it more. |
If someday you reopen this, it would be helpful to have examined code in real projects and found places (no pun intended) where trailing placeholders or a keyword placeholder would have improved the code, making it clearer, more obvious, and easier to maintain. Another suggestion is to show those proposed code edits to other users to see if they can figure out what it does (a readability test). Can a cold reader figure out that trailing placeholders and keyword argument placeholders are intended to force optional arguments to be required. Perhaps, poll a few programmers to see which of these three they would most want to see when reading or reviewing code:
For me at least, the last one has the least cognitive load. It is easy to write and isn't mysterious. Also, I would keep the flexibility of easily adding type annotations, changing the parameter names, and/or adding docstrings:
|
Feature or enhancement
Proposal:
1. Rationale
So, I considered automatic trimming of trailing
Placeholder
s instead of forbidding such. However it didn't sit well with me.So I thought this over. Not having trailing
Placeholders
works well, however both approaches of achieving this have flaws:a) Forbidding them - results in unexpected errors for the user.
b) Trimming automatically - obfuscates inner workings by implicit input modifications.
Furthermore, it sometimes could be desired to have a trailing
Placeholder
to convert argument to positional only.So I think it would be best not to have this restriction at all. This makes things more explicit and predictable. E.g. both would work:
And as expected,
self
ofp1
can be both positional or keyword, whileself
ofp2
becomes restricted to positional only.2. Implementation
This simplifies the code by not having to do error checks or trimming routines. Existing implementation handles trailing placeholders without modifications (except minor Fast Path fix in C).
3. Relation to
partialmethod
This has come out during #124788, but it turned out to be orthogonal.
partialmethod
stays as it was before - if no positionals providedargument kind
ofself
is unaffected, while ifargs
are provided thenself
becomes positional only.4. Performance
Slightly faster for both cases (with and without Placeholders) when merging args of previous
partial
with new args.I think this is more flexible, simple and "straight" (I.e. If base logic is understood, the implications are clear and behaviour is predictable).
Has this already been discussed elsewhere?
This is a minor feature, which does not need previous discussion elsewhere
Or to be more precise, this is an update for new unreleased functionality.
Links to previous discussion of this feature:
No response
Linked PRs
The text was updated successfully, but these errors were encountered: