Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-119127: functools.partial placeholders#119303
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
Could you add some tests? Or is that pointless without approval? |
I was thinking to wait for approval first. |
This comment was marked as outdated.
This comment was marked as outdated.
If this was accepted, the implementation is ready for review. Python and C versions are in sync and tests implemented. |
PRpython#119321 added a comment about the magic number bumpbut did not actually apply the new magic number.
…ython#119492)``_fancy_replace()`` is no longer recursive. and a single call does a worst-case linear number of ratio() computations instead of quadratic. This renders toothless a universe of pathological cases. Some inputs may produce different output, but that's rare, and I didn't find a case where the final diff appeared to be of materially worse quality. To the contrary, by refusing to even consider synching on lines "far apart", there was more easy-to-digest locality in the output.
Please open a new clean PR against main. Make sure the diff against main is correct before opening the PR. |
Thanks Erlend. |
Uh oh!
There was an error while loading.Please reload this page.
As I already had implementation I though PR might be helpful for others to see and evaluate.
From all the different extensions of
functools.partial
I think this one is the best. It is relatively simple and exposes all missing functionality. Otherpartial
extensions that I have seen lack functionality and would not provide complete argument ordering capabilities and/or are too complicated in relation to what they offer.Implementation can be summarised as follows:
a) All trailing placeholders are removed. (Makes things simpler)
b) Throws exception if not all placeholders are filled on call
c) retains optimization benefits of application on other
partial
instances.Performance penalty compared to current
functools.partial
is minimal for extension class. + 20-30 ns for initialisation and <4 ns when called with or without placeholders.To put it simply, new functionality extends
functools.partial
so that it has flexibility oflambda
/def
approach (in terms of argument ordering), but call overhead is 2x smaller.The way I see it is that this could only be justified if this extension provided completeness and no new functionality is going to be needed anywhere near in the future. I have thought about it and tried various alternatives and I think there is a good chance that this is the case. Personally, I don't think I would ever need anything more from
partial
class.Current implementation functions reliably.