Skip to content
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

Partial method argument order #453

Closed
qqfunc opened this issue Apr 28, 2024 · 5 comments
Closed

Partial method argument order #453

qqfunc opened this issue Apr 28, 2024 · 5 comments
Labels
enhancement New features, or improvements to existing features.

Comments

@qqfunc
Copy link
Contributor

qqfunc commented Apr 28, 2024

What is the problem or limitation you are having?

As described in #26, when there is an Objective-C class that has someMethod:arg1:arg2: and someMethod:arg2:arg1: methods...

>>> obj.someMethod(..., arg1=..., arg2=...)
>>> obj.someMethod(..., arg2=..., arg1=...)

These call the same method (someMethod:arg1:arg2:). This seems to be because partial methods only store a single order of arguments.

Describe the solution you'd like

Allow them to be distinguished. (Then non-existent argument orders may no longer be allowed.) The argument names of partial methods are stored using dictionaries whose keys are frozenset. And, the values of the dictionaries are like (selector string, [argument1 name, argument2 name, ...]). In this case, storing multiple selectors and lists may be a solution.

Describe alternatives you've considered

Store no argument names. Then, use respondsToSelector: in ObjCPartialMethod to check if the method of specified name exists. (I feel there are some other ways...)

Additional context

No response

@qqfunc qqfunc added the enhancement New features, or improvements to existing features. label Apr 28, 2024
@freakboy3742
Copy link
Member

Is this a problem you're having with an actual API? If so - which one? I'm a little surprised that Apple would construct an API where the argument order is the discriminator between method forms.

Regardless, I think you've missed another possible option for implementation. ObjCPartial currently uses frozenset to track the options on an ObjCPartialMethod. If that were to be replaced with an "order preserving set" - i.e., more of a "tuple, but inserting the same item is prohibited" - then arg1:arg2: and arg2:arg1 should hash differently, and thus yield different lookups.

In fact, we might even be able to combine this with a fix for #148 - using a literal tuple would be a key part of that fix, with the other part being post-processing the kwargs to remove __ suffixes.

@qqfunc
Copy link
Contributor Author

qqfunc commented Apr 29, 2024

I think this is only if I implement such methods myself. I understand such a situation won't be a problem in almost cases. Its priority would be low.

By the way, I thought the use of frozenset was for performance since the similar list is also contained in dictionary values. Is this wrong? Also, does the way of using tuple as keys mean that the list of the argument names is no longer necessary?

@freakboy3742
Copy link
Member

The use of frozenset is partially for performance; but at least as currently implemented, it's also being used for the uniqueness properties of set. At the time that code was introduced, dictionary order (and thus kwargs order) couldn't be guaranteed. As of Python 3.6, we can now guarantee kwargs order, so that isn't a concern.

And yes, the list is serving almost exactly the same purpose, so there's very likely an optimisation to be had simplifying ObjCPartialMethod.methods to be a simple lookup of order->name, rather than frozenset(order)->(name, order).

Interestingly, the code includes a reference to the exact behavior you're describing here - along with a TODO to clean it up once Python 3.6 is deprecated :-)

@qqfunc
Copy link
Contributor Author

qqfunc commented Apr 29, 2024

Thank you. That seems to be true.

@freakboy3742
Copy link
Member

Fixed by #462.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features, or improvements to existing features.
Projects
None yet
Development

No branches or pull requests

2 participants