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

Add mechanism for invoking methods with duplicated argument names. #148

Closed
freakboy3742 opened this issue Feb 16, 2020 · 10 comments
Closed
Labels
enhancement New features, or improvements to existing features.

Comments

@freakboy3742
Copy link
Member

freakboy3742 commented Feb 16, 2020

Objective C arguments are all named, but ordered. As a result, it is entirely legal for argument names to be duplicated. For example, NSLayoutConstraint has a method +constraintWithItem:attribute:relatedBy:toItem:attribute:multiplier:constant: - the attribute argument name is duplicated.

This is a problem for Rubicon, because the obvious mapping to keyword arguments wont work:

NSLayoutConstraint.constraintWithItem(
    item,
    attribute=...,
    relatedBy=...
    toItem=...
    attribute=...
    multiplier=...
    constant=...
)

These methods can be accessed using the "older" underscore-based API mapping:

NSLayoutConstraint.constraintWithItem_attribute_relatedBy_toItem_attribute_multiplier_constant_(...)

but this will almost always violate line length constraints.

We need a way to the more Pythonic invocation style on methods that have duplicated arguments.

@freakboy3742
Copy link
Member Author

Some ideas:

  1. Argument suffixing. Look for a double underscore in any argument name, and strip anything after the double underscore.
NSLayoutConstraint.constraintWithItem(
    item,
    attribute__from=...,
    relatedBy=...
    toItem=...
    attribute__to=...
    multiplier=...
    constant=...
)

Downsides: you won't be able to invoke any argument that actually has a double underscore in it's name. Objective C tends towards camelCase naming, though, so this is probably a low risk.

  1. Explicit aliasing. At some point in the declaration process, provide a way to map keywords provided in Python to keywords used in the call.
NSLayoutConstraint.constraintWithItem.set_argument_mapping(
    fromAttribute='attribute',
    toAttribute='attribute', 
)

NSLayoutConstraint.constraintWithItem(
    item,
    fromAttribute=...,
    relatedBy=...
    toItem=...
    toAttribute=...
    multiplier=...
    constant=...
)

This allows for an explicit mapping of names - or even a complete re-mapping of names. For example, you could completely change the API to use snake case, rather than camelCase:

NSLayoutConstraint.constraintWithItem.set_argument_mapping(
    attr='attribute',
    related_by='relatedBy',
    to_item='toItem',
    to_attr='attribute',     
)

NSLayoutConstraint.constraintWithItem(
    item,
    attr=...,
    related_by=...
    to_item=...
    to_attr=...
    multiplier=...
    constant=...
)

The downside to this approach is that the mapping from Objective C documentation to Python documentation isn't completely transparent - you need to provide documentation for how Python maps each specific affected API.

@freakboy3742 freakboy3742 added enhancement New features, or improvements to existing features. up-for-grabs labels Feb 16, 2020
@dgelessus
Copy link
Collaborator

  1. Argument suffixing. Look for a double underscore in any argument name, and strip anything after the double underscore.

I like this one for the most part - it's very simple (doesn't need any manual declarations) and it reads nicely IMHO. I see three issues with this variant though:

  1. Allowing users to freely choose what to put after the double underscores will likely lead to inconsistencies between developers/codebases. In the original Objective-C method declaration, the two argument keywords are identical, so developers need to come up with their own terms to differentiate the two arguments. Different developers will probably think of different names, and you end up with different identifiers for the same method across different projects (or even in the same project, when multiple developers are involved).
  2. Somewhat related to the previous point: Someone who reads this code and doesn't already know the meaning of the double-underscore suffixes might think that from and to refer to something declared elsewhere (in Objective-C or Python), or that these specific names have a special meaning to Rubicon. In fact, the suffixes are completely arbitrary and have no meaning (except that they have to be different from each other).
  3. Before Python 3.6, the order of keyword arguments was not retained in the dictionary received by a **kwargs function. This makes it impossible to tell which order attribute__from or attribute__to are supposed to have.

A possible solution for both of these issues would be to use numbers as suffixes (i. e. attribute__0/attribute__1 or attribute/attribute__1 instead of attribute__from/attribute__to). This would make the ordering unambiguous even on older Python versions with no ordered **kwargs, it would make the translation from Objective-C to Python unambiguous/deterministic (leading to consistent naming across projects), and IMHO it makes it clearer that the suffixes are just there to resolve a naming conflict and have no further meaning.

  1. Explicit aliasing. At some point in the declaration process, provide a way to map keywords provided in Python to keywords used in the call.

I'm not a huge fan of this solution - I'd prefer to avoid manual declarations where possible, because they are less convenient to write and more confusing to read. My first point regarding the previous solution (allowing free choice of names leads to unnecessary naming differences) also applies here.

This allows for an explicit mapping of names - or even a complete re-mapping of names. For example, you could completely change the API to use snake case, rather than camelCase

I don't think this is a good idea - if you start doing that, you would need to be consistent and write declarations like that for every Objective-C method you use, which simply takes too much work for a very small benefit.

In general I think there are limits to how much Rubicon should try to make Objective-C APIs look like Python. Of course we want to make it as convenient as possible to interface between Python and Objective-C, so we implement some automatic conversions, wrapper classes, operators, etc. to make the Python-side calls less verbose. But in the end, the user still needs to remember that they're interfacing with a different language. A proper Python API around an Objective-C class/library needs to do much more than just changing names from camelCase to snake_case. I think supporting this kind of renaming natively in Rubicon would just give people false ideas that properly wrapping an API takes less work than it actually does.

@dgelessus
Copy link
Collaborator

By the way, as a workaround for the line length issue, you can use the send_message function. In that case the method name is passed in as a string, so you can wrap it across lines:

send_message(
    NSLayoutConstraint,
    (
        "constraintWithItem:attribute:relatedBy:"
        + "toItem:attribute:multiplier:constant:"
    ),
    ..., # the actual arguments
)

This doesn't look very nice and I think we should find a better solution, but it would be an acceptable workaround for now if line length is an issue.

@dgelessus
Copy link
Collaborator

Also, here's another possible solution, using a terrible syntax hack:

send_message[NSLayoutConstraint,
    "constraintWithItem":item1,
    "attribute":attr1,
    "relatedBy":rel,
    "toItem":item2,
    "attribute":attr2,
    "multiplier":mult,
    "constant":const,
]

You see, this looks like Objective-C's method call syntax, with the square brackets and colons and all. :P

This works because of Python's slice syntax (seq[12:34]). The above code is equivalent to:

send_message[(NSLayoutConstraint,
    slice("constraintWithItem", item1),
    slice("attribute", attr1),
    slice("relatedBy", rel),
    slice("toItem", item2),
    slice("attribute", attr2),
    slice("multiplier", mult),
    slice("constant", const),
)]

I haven't decided yet if this idea is good or bad, but it would definitely work.

@freakboy3742
Copy link
Member Author

I generally agree with your analysis regarding (1) vs (2). There's also the added bonus that (1) should be a lot easier to implement :-)

Regarding the specifics of (1): The dictionary ordering problem you mention is almost a non-issue; Python 3.5 reaches end of life in October.

I can see the point you're making regarding the naming convention for post-underscore text; however, I'm not convinced it's as big an issue as you make out. No matter what suffix is used, an end user who encounters the double underscore syntax will need to go looking for details on how to decode what it means, because it the name won't map in obvious ways to the Objective C documentation. As long as we make the documentation for the double underscore convention prominent, it's easy to impart the interpretation rule that the suffixes will be be ignored, and users should stop looking for any deep meaning, and internalize an "ignore suffix" reading behavior.

We could possibly suggest that absent of any better convention, integers make a good suffix; and/or, if necessary for Python 3.5 support, use sorting of suffixes to provide order disambiguation (with 'no suffix' being sorted first).

@dgelessus
Copy link
Collaborator

The dictionary ordering problem you mention is almost a non-issue; Python 3.5 reaches end of life in October.

Good point - we could require at least Python 3.6 for this specific feature, or even decide to drop 3.5 early. (I don't expect there to be many Rubicon users who still need 3.5 specifically.)

I can see the point you're making regarding the naming convention for post-underscore text; however, I'm not convinced it's as big an issue as you make out.

That's true - the syntax isn't fully self-explanatory either way, and we definitely need to document this feature in the usual places (how-to and reference docs). Hopefully the double-underscore syntax is unusually-looking enough that people will go look at the Rubicon docs rather than searching for the exact identifier in Apple's documentation.

@mhsmith
Copy link
Member

mhsmith commented Apr 10, 2023

I think enforcing integer suffixes is better than allowing a free choice, both for these reasons:

it would make the translation from Objective-C to Python unambiguous/deterministic (leading to consistent naming across projects), and IMHO it makes it clearer that the suffixes are just there to resolve a naming conflict and have no further meaning.

... and also because it would allow the dict used in a **kwargs call to be built up in any order. For the meaning of the call to be sensitive to the dict insertion order would be extremely un-Pythonic, and hard to debug.

@freakboy3742
Copy link
Member Author

Being sensitive to dict insertion order is definitely a concern; but I don't know that I'd consider it a particularly high priority. At the point you're interfacing with an Objective C API, I don't think it would be reasonable to assume that all Pythonic invocation conventions (such as invoking a method with **kwargs) will be honoured transparently. Given that insertion order is something that Python does have, I'd almost consider that dict insertion order would be what we should preserve, rather than trying to re-sort some keys in the dictionary based on an interpretation like lexical ordering of a suffix.

qqfunc added a commit to qqfunc/rubicon-objc that referenced this issue Apr 30, 2024
@qqfunc qqfunc mentioned this issue Apr 30, 2024
4 tasks
@freakboy3742
Copy link
Member Author

Fixed by #462.

@mhsmith
Copy link
Member

mhsmith commented Jul 1, 2024

enforcing integer suffixes [...] would allow the dict used in a **kwargs call to be built up in any order

I was wrong about this, because as @qqfunc pointed out in #453, Objective-C method calls may be sensitive to argument order even if they don't have repeated argument names.

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

3 participants