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

Proposal for nicer init() #26

Open
dimaqq opened this issue Feb 17, 2017 · 19 comments
Open

Proposal for nicer init() #26

dimaqq opened this issue Feb 17, 2017 · 19 comments
Labels
enhancement New features, or improvements to existing features.

Comments

@dimaqq
Copy link

dimaqq commented Feb 17, 2017

Through trial and error I got following to pass:

tmp = ObjCClass("UINavigationController").alloc().initWithRootViewController_(root)

Wouldn't it be nice to get that data from call signature:

tmp = ObjCClass("UINavigationController").alloc().init(rootViewController=root)

And then, perhaps, move to:

UINavigationController = ObjCClass("UINavigationController")
tmp = UINavigationController(rootViewController=root)
@dgelessus
Copy link
Collaborator

Pythonista's objc_util module supports calling Objective-C methods using keyword arguments like this, and it definitely helps with readability. Sounds like a good idea to me.

This would also be a good use case for PEP 468's order-preserving keyword arguments, which are available as of Python 3.6. Ordered keyword arguments are not necessary for this feature though, they are just an optimization - on older Python versions, the correct method name would need to be guessed, by trying every possible permutation of the keyword arguments, until a valid method name is found.

@freakboy3742
Copy link
Member

Completely agreed that this sounds like a good idea - it's just a matter of implementation. Since argument order isn't guaranteed (until Python 3.6, anyway), that means the lookup process will be (a) O(2^n) complexity for the number of arguments, and (b) potentially ambiguous, because the method method(arga=1, argb=2) will match methodWithArga:Argb: and methodWithArgb:Arga:.

I'm not sure if the latter will be a problem in practice - and it becomes a non-issue under Python3.6 - but it's worth keeping in mind.

There's also a minor issue of how initWithArgA:ArgB maps to a method call; it would be nice if the method was init(argA=..., argB=...); but we might need to compromise with initWith(argA=..., argB=...) - unless of course, the "with" becomes a magic keyword in method names.

@dgelessus
Copy link
Collaborator

To keep it simple at first, maybe it would be good to require initWithArgA(..., argB=...). Then the only thing that needs to be done is inserting the :s after each selector part (after initWithArgA and argB). More advanced "name guessing" could be added later. It would also halve the "guessing time" on Pythons with unordered kwargs.

@dgelessus
Copy link
Collaborator

Quick update - I've played around with this a little and implemented a basic version of this feature (in my local copy of the repo). The current implementation is not very good though. Right now, when any nonexistant attribute is accessed on an ObjCInstance, it is always interpreted as a method call with kwargs, even if there is no method that starts with that name. This breaks a couple of tests that require nonexistant attributes to raise an AttributeError. I also think it's not a good solution to magically make every attribute exist.

As a (hopefully better) alternative I'll try caching all instance methods when an ObjCClass is created. This way, we know in advance which method names exist, and can create attributes for those specifically. I don't know how well this will perform, since the entire method list of the class and all superclasses needs to be looked through. At least it only needs to be done once per class, so if there is a noticeable performance impact, it will only be when each class is first loaded.

@dgelessus
Copy link
Collaborator

Okay, PR has been made. The way I implemented it now uses sets as dictionary keys to look up the method for a set of kwargs. This doesn't depend on PEP 468, so it should work equally well on all Python versions. I haven't tested the performance of the new syntax much, but the unit tests run in about the same time as before, so hopefully it shouldn't affect the speed of existing code much.

@dgelessus dgelessus added the enhancement New features, or improvements to existing features. label Mar 18, 2017
@dimaqq
Copy link
Author

dimaqq commented Jul 5, 2017

So, now that PR is merged, what's the new way to do the following?

UIWindow.alloc().initWithFrame_(UIScreen.mainScreen.bounds)

Or are we not there yet?

@freakboy3742
Copy link
Member

@dimaqq The call would be:

UIWindow.alloc().initWithFrame(UIScreen.mainScreen.bounds)

The first argument is always a positional argument; all subsequent arguments are keywords.

@dimaqq
Copy link
Author

dimaqq commented Jul 6, 2017

Oh, cool, I didn't realise that.

Would be nice to have that documented :)

Here's what I can do now (it works, I hope I understood why):

-            rv = view.dequeueReusableCellWithReuseIdentifier_forIndexPath_("knob", path)
+            rv = view.dequeueReusableCellWithReuseIdentifier("knob", forIndexPath=path)

@freakboy3742
Copy link
Member

Documentation is definitely needed (but that's true of most of Rubicon...) - but AFAICT, this has now been implemented. If I'm mistaken, please re-open this ticket.

@dgelessus
Copy link
Collaborator

Reopening this for now - there are two suggestions from the OP that are not yet supported:

  • Leaving out the with parts of the init selector: DGFoo.alloc().init(foo="spam", bar="ham") translates to [[DGFoo alloc] initWithFoo:@"spam", bar:@"ham"]
  • Leaving out the explicit .alloc().init(...) entirely: DGFoo(foo="spam", bar="ham") also translates to the above

IMHO both of these would be good to have as they would make object creation shorter and more readable. This is also basically what Swift does when it imports init methods from Objective-C.

@qqfunc
Copy link
Contributor

qqfunc commented Apr 29, 2024

For the NSObject() style, how about checking if it is used in a subclass of NSObject? (Related to this comment in #256)

>>> ObjCInstance(ptr)  # Wrap a Objective-C object
>>> NSObject(...)  # Create a new instance of NSObject
>>> NSObject(ptr)  # Currently allowed, but not allowed in this idea

@freakboy3742
Copy link
Member

The potential issue with what you've described is that when you wrap an ObjCInstance, you likely want to preserve the Python details of the class you're wrapping. This might be possible with the approach you've described, but it's firmly in the space of "provide at least a prototype implementation that works, not just a hypothetical potential syntax".

Personally, I suspect any movement on this issue will require a resolution to #70.

@qqfunc
Copy link
Contributor

qqfunc commented Apr 29, 2024

Do you mean to make NSObject instances actual instances of ObjCClass("NSObject")? (I'm not sure I understand your opinion correctly.)

@freakboy3742
Copy link
Member

I mean that, at present, an instance of ObjCClass is an ObjCInstance, not a "type". Untangling the hierarchy of relationships between ObjCClass, ObjCInstance, and type and will be at least part of the complication in trying to make constructors behave in the way described by this proposal.

@qqfunc
Copy link
Contributor

qqfunc commented Apr 29, 2024

Indeed, this currently seems to be weird a bit. ObjCInstance is generated by ObjCClass, but ObjCClass is a subclass of ObjCInstance. If I understand correctly, ObjCInstance should be instance of ObjCClass in this case.

class ObjCInstance(metaclass=ObjCClass):
    ...

Then, ObjCClass would also need to implement __getattr__ and so on because ObjCInstance is no longer a subclass of ObjCClass.
Sorry to be repeat myself, but is this understanding correct?

@freakboy3742
Copy link
Member

That is a possible end state; whether that end state is desirable is another question. What we have currently is another end state - one that we know works.

This is why I made my comment about needing a prototype - this isn't a conversation that will be improved by hypothetical discussions about possible syntax unless those discussions also include prototype implementations that demonstrate that the proposed syntax is actually possible in practice.

@qqfunc
Copy link
Contributor

qqfunc commented Apr 30, 2024

Thank you so much! Now I could understand what you were saying. I will keep this in mind as a future issue.

@mhsmith
Copy link
Member

mhsmith commented Nov 24, 2024

Saving some notes from #256 (comment), since that issue will hopefully be closed soon:

The Pythonic constructor syntax is complicated by the pattern from beeware/toga#2468, where a failed init releases the object and returns NULL. For example, NSImage(contentsOfFile=path) would need to do the following:

  • Call NSImage.alloc, and wrap it in an ObjCInstance.
  • CallinitWithContentsOfFile
  • If initWithContentsOfFile returns NULL, then call retain to cancel out the release in ObjCInstance__del__, and throw an exception.

Then, as long as we can assume that the plain init method always takes zero arguments, that could be called by writing NSImage().

This may clash with the way the Python constructor is currently being used to wrap an existing Objective-C object, but it should be possible to distinguish that mode by checking argument types, or by passing a keyword argument like _rubicon_something.

@mhsmith
Copy link
Member

mhsmith commented Nov 26, 2024

init may also return a different Objective-C object to the one it was called on – #543 (comment).

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

5 participants