ronaldoussoren / pyobjc

The Python <-> Objective-C Bridge with bindings for macOS frameworks
https://pyobjc.readthedocs.io
523 stars 45 forks source link

pyobjc==10.3 breakage #610

Open justvanrossum opened 1 month ago

justvanrossum commented 1 month ago

Describe the bug

There's a regression for a code pattern like this:

import AppKit

class TestClass(AppKit.NSObject):

    def __new__(cls):
        print("in __new__")
        return cls.alloc().init()

    def __init__(self):
        print("in __init__")

inst = TestClass()

With pyobjc==10.2 this outputs this:

in __new__
in __init__

However, with pyobjc==10.3, __init__ doesn't get called, and this is the output:

in __new__
ronaldoussoren commented 1 month ago

What's your use case for this?

I've disabled calling __init__ because that leads to more consistent behaviour, especially when considering initialiser with NSError** arguments, e.g.


class MyDocument(NSDocument):
     def __init__(self, *args, **kwds): pass

document = MyDocument()   # __init__ gets called
document, error = MyDocument(type="mytype", error=None). # __init__ does not get called

I've chosen to disable __init__ wholesale for this reason and because the right way to initialise ObjC subclasses is to use an init* method. But I will reconsider if there's a clear use case for using __init__.

justvanrossum commented 1 month ago

My use case was this: I'd like to instantiate my NSObject subclass as if it's a Python class, by just calling it with arguments.

ronaldoussoren commented 1 month ago

That's already possible, but you have to implement one or more init methods for this. What doesn't work is writing a single intermediate subclass and than implemented its classes as if they are plain python classes. TBH, that's not something I had considered.

The changelog mentions this a bit too concisely, https://pyobjc.readthedocs.io/en/latest/notes/instantiating.html is a bit clearer.


class MyObject(NSObject):
    init = None  # Calling MyOjbect() is not allowed

    def initWithX_y_(self, x_value, y_value):
        self = super.init()
        self.x = x_value
        self.y = y_value
        return self

value = MyValue(x=42, y=24)

One limitation is that the order or keywords is currently important. I'm not convinced that this is the right design, but it was easier to start with this limitation than to introduce it later when I run into system frameworks where the order is important. I'll probably drop the restriction during the summer, but only after I've checked the framework bindings and have hammered out a design for a similar feature for calling other methods in a nicer way.

justvanrossum commented 1 month ago

But what about the aspect of the 10.3 update unnecessarily breaking working code?

ronaldoussoren commented 1 month ago

I'll revert the change that broke __init__ and will reconsider for 11.0.

justvanrossum commented 1 month ago

Idea: only call __init__ if the class defines its own __new__?

ronaldoussoren commented 1 month ago

Idea: only call __init__ if the class defines its own __new__?

I like the idea, but have to check how hard it will be to implement this in the current setup. In a very real sense every class now implements its own __new__ when checking from __call__ due to the way the new functionality is implemented. That's not strictly necessary, but made it easier to ensure that pydoc works for introspecting the the signature for __new__.

That said, I already detect if a user has written the __new__ implementation for other reasons and it should be possible to do this in the implementation of __call__ as well.

justvanrossum commented 1 month ago

For completeness sake: the __init__ change in 10.3 breaks any code that uses the cocoa-vanilla library. This includes applications like DrawBot, FontGoggles, RoboFont and many extensions written for GlyphsApp.

ronaldoussoren commented 4 weeks ago

Thanks for the reference to cocoa-vanilla, I'll test my changes with that library as well.

I shortly commit some changes that ensure that the following tests pass:


class OC_DunderInitBase(NSObject):
    # Helper for ``TestUsingDunderInit``
    def __new__(cls, *args, **kwds):
        return cls.alloc().init()

class TestUsingDunderInit(TestCase):
    # Some users have an intermediate class
    # which implements ``__new__`` to be able
    # to create Cocoa sublcasses using a similar
    # interface as normal Python subclasses, e.g.
    # with ``__init__`` for initializing the instance.
    #
    # This should continue to work.

    def test_using_dunder_init(self):
        class OC_DunderInitSub1(OC_DunderInitBase):
            def __init__(self, x, y=2):
                self.x = x
                self.y = y

        o = OC_DunderInitSub1(x=1)
        self.assertIsInstance(o, OC_DunderInitSub1)
        self.assertEqual(o.x, 1)
        self.assertEqual(o.y, 2)

        with self.assertRaises(TypeError):
            OC_DunderInitSub1()

        with self.assertRaises(TypeError):
            OC_DunderInitSub1(9, z=4)

    def test_multipe_generations(self):
        class OC_DunderInitSub2(OC_DunderInitBase):
            def __init__(self, x, y):
                self.x = x
                self.y = y

        class OC_DunderInitSub3(OC_DunderInitSub2):
            def __init__(self, x, y, z):
                super().__init__(x, y)
                self.z = z

        o = OC_DunderInitSub3(1, 2, 3)
        self.assertIsInstance(o, OC_DunderInitSub3)
        self.assertEqual(o.x, 1)
        self.assertEqual(o.y, 2)
        self.assertEqual(o.z, 3)

That is, __init__ works again when using an explicit __new__ method in a class or one of its superclasses.

ronaldoussoren commented 4 weeks ago

I've clicked around a little in python3 -m vanilla.test.testAll and that code appears to work with this update.

I did have a crash when using vanillaBrowser, but that seems to be related to Python 3.12 support in cocoa-vanilla:

Traceback (most recent call last):
  File "/Users/ronald/Projects/pyobjc-8/workenv/lib/python3.12/site-packages/vanilla/test/testAll.py", line 612, in openTestCallback
    BrowserTest(self.w.drawGrid.get())
  File "/Users/ronald/Projects/pyobjc-8/workenv/lib/python3.12/site-packages/vanilla/test/testAll.py", line 391, in __init__
    self.w.open()
  File "/Users/ronald/Projects/pyobjc-8/workenv/lib/python3.12/site-packages/vanilla/vanillaWindows.py", line 269, in open
    self.show()
  File "/Users/ronald/Projects/pyobjc-8/workenv/lib/python3.12/site-packages/vanilla/vanillaWindows.py", line 310, in show
    self._window.makeKeyAndOrderFront_(None)
  File "/Users/ronald/Projects/pyobjc-8/workenv/lib/python3.12/site-packages/vanilla/vanillaBrowser.py", line 112, in outlineView_child_ofItem_
    return item.getChild(child)
           ^^^^^^^^^^^^^^^^^^^^
  File "/Users/ronald/Projects/pyobjc-8/workenv/lib/python3.12/site-packages/vanilla/vanillaBrowser.py", line 276, in getChild
    childObj = self.__class__(name, obj, self.object, setter)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ronald/Projects/pyobjc-8/workenv/lib/python3.12/site-packages/vanilla/vanillaBrowser.py", line 246, in __init__
    self.arguments = getArguments(getattr(obj, "__init__"))
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ronald/Projects/pyobjc-8/workenv/lib/python3.12/site-packages/vanilla/vanillaBrowser.py", line 166, in getArguments
    arguments = inspect.formatargspec(*inspect.getargspec(obj))
                ^^^^^^^^^^^^^^^^^^^^^
AttributeError: module 'inspect' has no attribute 'formatargspec'. Did you mean: 'formatargvalues'?
ronaldoussoren commented 4 weeks ago

There will be a release of 10.3.1 later this weekend unless I run into unexpected issues when running the full test suite using the latest python updates.

justvanrossum commented 4 weeks ago

Thank you!

schriftgestalt commented 4 weeks ago

Just for the record, the Glyphs.app python wrapper uses a pattern like this to make the objc API more pythonic:

def __GSNode__new__(typ, *args, **kwargs):
    return typ.alloc().init()

GSNode.__new__ = python_method(__GSNode__new__)

def __GSNode__init__(self, pt: Optional[NSPoint] = None, type: Optional[int] = None, x: Optional[float] = None, y: Optional[float] = None, name: Optional[str] = None) -> None:
    if pt:
        self.setPosition_(pt)
    elif x is not None and y is not None:
        self.setPosition_((x, y))
    if type:
        self.type = type
    if name:
        self.name = name

GSNode.__init__ = python_method(__GSNode__init__)
ronaldoussoren commented 3 weeks ago

There will be a release of 10.3.1 later this weekend unless I run into unexpected issues when running the full test suite using the latest python updates.

... or unless my build VM crashes. I've restarted a test run and expect to release later today.

ronaldoussoren commented 3 weeks ago

Just for the record, the Glyphs.app python wrapper uses a pattern like this to make the objc API more pythonic:

def __GSNode__new__(typ, *args, **kwargs):
  return typ.alloc().init()

GSNode.__new__ = python_method(__GSNode__new__)

def __GSNode__init__(self, pt: Optional[NSPoint] = None, type: Optional[int] = None, x: Optional[float] = None, y: Optional[float] = None, name: Optional[str] = None) -> None:
  if pt:
      self.setPosition_(pt)
  elif x is not None and y is not None:
      self.setPosition_((x, y))
  if type:
      self.type = type
  if name:
      self.name = name

GSNode.__init__ = python_method(__GSNode__init__)

That should work again in 10.3.1. The change in 10.3.1 is to only disable calling __init__ when __new__ is the generic implementation that I added in 10.3.

ronaldoussoren commented 3 weeks ago

PyObjC 10.3.1 is now available on PyPI with this fix.

justvanrossum commented 3 weeks ago

I can confirm that my code works again with the 10.3.1 update. Thank you so much!

schriftgestalt commented 2 weeks ago

I finally got to test this with my code. And it is still not working as before. I added some "convenience" to often used Cocoa classes like this:

NSMenuItem.__new__ = staticmethod(__GSObject__new__)

def __NSMenuItem__init__(self, title, callback=None, target=None, keyboard=None, modifier=0):
    # actual do stuff here, remove for clarity.

NSMenuItem.__init__ = python_method(__NSMenuItem__init__)

I get this error:

  File "_new.py", line 98, in __call__
    return self._function(*args, **kwds)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "_new.py", line 133, in __new__
    raise TypeError(
TypeError: NSMenuItem() does not accept positional arguments

And if I do

GSSuperClass.__new__ = staticmethod(__GSObject__new__)
GSSubClass.__init__ =  = python_method(__ GSSubClass__init__)

GSSubClass(someArgument)

it fails, too. I need to add:

GSSubClass.__new__ = staticmethod(__GSObject__new__)

I can deal with second problem, but it would be great to be able to amend Cocoa classes like this.

ronaldoussoren commented 2 weeks ago

I'm trying to reproduce the first problem, but so far without success. The traceback indicates that my __new__ implementation is used and not __GSObject__new__.

Is the code below similar enough to what you're doing?

from Foundation import NSURL
from objc import python_method

def override__new__(self, *args, **kwds):
    return self.alloc().initWithString_("https://www.python.org")

def override__init__(self, *args, **kwds):
    print(self, args, kwds)

NSURL.__new__ = staticmethod(override__new__)
NSURL.__init__ = python_method(override__init__)

o = NSURL()
print(o, type(o))

The second issue is a consequence of how the new feature is implemented: Every (native) class gets its own __new__ to ensure that it has a docstring that reflects the actual interface (that is, help(NSURL) includes information about the supported keyword arguments for that particular class.

It might be possible to avoid the second problem in your code by carefully calling objc.lookUpClass for all super classes and set their __new__ before doing anything that might resolve subclasses. That's inherently fragile though.

I'm afraid there's no good solution for the second problem, other then giving up on good help on my end or explicitly setting __new__ for all native classes on your end.

schriftgestalt commented 2 weeks ago

I can fix the second problem on my end. I just need to copy paste that line a few times. So don’t worry about it.

NSURL is not a good sample, it would need to parse the args in __new__, here again with the NSMenuItem and intended usage (having an argument when initializing):

from AppKit import NSMenuItem
from objc import python_method

def override__new__(self, *args, **kwds):
    return self.alloc().init()

def override__init__(self, path, action=None, target=None):
    self.setTitle_(path)
    if action:
        self.setAction_(action)
    if target:
        self.setTarget_(target)

NSMenuItem.__new__ = staticmethod(override__new__)
NSMenuItem.__init__ = python_method(override__init__)

o = NSMenuItem("Menu Title")
print(o, type(o))

The NSURL could be "wrapped" like this:

from Foundation import NSURL
from objc import python_method

def override__new__(self, *args, **kwds):
    return self.alloc().initWithString_(args[0])

NSURL.__new__ = staticmethod(override__new__)

o = NSURL("https://www.python.org")
print(o, type(o))
ronaldoussoren commented 2 weeks ago

The annoying bit is that both work for me. That is, given a virtualenv named "pydotorg" in which I installed PyObjC 10.3.1 from PyPI ("pip install pyobjc"):

% ./pydotorg/bin/python repro.py
<NSMenuItem: 0x600002a79340 Menu Title> <objective-c class NSMenuItem at 0x1ed4a0930>

The current development version (what will be 11.0 later this year) behaves the same. I do get a TypeError when I remove the assignment of NSMenuItem.__new__.

schriftgestalt commented 2 weeks ago

You are right. It works when run in plain python. But it doesn't when run inside my app. I’ll dig a bit.

schriftgestalt commented 2 weeks ago

I have narrowed it down. It seems only to happen when two python files are executed in the same context. (e.g. two plugins that are based on py2app running inside my app)

Both do:

from AppKit import NSMenuItem
from myWrapper import something # this contains the above code that assigns the methods to NSMenuItem.

The first time first time all is fine, NSMenuItem is imported, and then the myWrapper is loaded and the methods are assigned. But when the second file imports NSMenuItem, the import seems to overwrite my __new__ with the default implementation. The following import from my module doesn't assign my __new__ again as that is only executed when the module is initially loaded.