Remove metaclass usage

[Jerakin]

"Fixed" how the metaclass is defined, and later used.

Using the __metaclass__ is an old pattern. The keyword argument is standard in python 3

[hannesdelbeke]

@copilot review this and provide pros cons and feedback summary

[hannesdelbeke]

codex review

This change introduces a metaclass conflict: QApplication (PySide6/Shiboken type) already has a custom metaclass, so class BlenderApplication(QApplication, metaclass=ABCMeta) is likely to fail at import/class creation with TypeError: metaclass conflict.

metaclass = ABCMeta inside init was ineffective, but replacing it with metaclass=ABCMeta on a Qt base class is not safe.

Suggested fix: keep the Qt metaclass and enforce abstract behavior another way (for example, runtime checks in init_subclass, or a combined metaclass that inherits both the Qt metaclass and ABCMeta).

[hannesdelbeke]

hmm seems i need to investigate this more, will revisit this

[Jerakin]

I can confirm that this is sometimes not working. Tested it on a different computer and I do get TypeError: metaclass conflict: on that one.

If ABCs are not working with Qt, we should probably remove it and simple raise NotImplementedError in any functions that needs to be implemented in when subclassing. Not as elegant as you will only get the error when the code is hit, not when instantiating the class.

But bqt isn't a library in that kind of way so we the chance of someone else trying to do their own BlenderApplication implementation is low

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[hannesdelbeke]

what is the benefit if this PR?
it seems to introduce risk.
is there something not working atm, or is it only future proofing?

[Jerakin]

Code cleanup, aligning expectations.

The statement __metaclass__ = ABCMeta doesn't actually do anything (in python3).

It's visible in an IDEA like this

But most importantly this means that any use of @abstractmethod will not do what's expected. It's expected that all abstractmethods needs to be implemented in subclasses or you will not be allowed to instantiate it.

Running this code here will raise an error, which is expected.
TypeError: Can't instantiate abstract class App without an implementation for abstract method 'fnc'

from abc import ABCMeta, abstractmethod

class Base(metaclass=ABCMeta):
    @abstractmethod
    def fnc(self):
        pass

class App(Base):
    pass

App()

This is the main purpose of ABCs. However, this is doesn't happen in bqt. We know this as even though _get_application_icon is an abstractmethod in BlenderApplication, Win32BlenderApplication doesn't implement it. This should raise an error but it doesn't.

[hannesdelbeke]

This should raise an error but it doesn't.

is that because qt already implements it, and we override the ones we need? so there are no missing ones

[hannesdelbeke]

just had a look at the code again, it completely changed from first commit, lot less complex now.
doesn't feel as risky now. i ll want to test this one myself though to make sure since it's the core of bqt

[Jerakin]

This should raise an error but it doesn't.

is that because qt already implements it, and we override the ones we need? so there are no missing ones

It's important to note that with abstractmethod there are not "the ones we need". Any method that is marked with abstractmetod is required to be implemented. If not all abstractmethods are implemented it will raise an error.

The statement __metaclass__ = ABCMeta has no effect. If it did have an effect it would as Copilot say probably crash.

Effectively the code is this, it doesn't use ABC but defines abstractmethod(s).

from abc import abstractmethod

class Base:
    @abstractmethod
    def fnc(self):
        pass

class App(Base):
    pass

App()

Your IDEA doesn't do any runtime checks, but it knows that the parent class (Base) defines some abstractmethods so it will warn on "App" that we are not implementing all abstractmethods. This could give you the illusion that it is properly defining the ABC.