magicgui icon indicating copy to clipboard operation
magicgui copied to clipboard

Better type checking integration for `my_gui_class.gui`

Open multimeric opened this issue 1 year ago • 2 comments

Consider this example:

from magicgui.experimental import guiclass

@guiclass
class MyDataclass:
    number: int = 0

if __name__ == '__main__':
    obj = MyDataclass()
    obj.gui.show(run=True)

Running pyright widget.py gives:

widget.py:10:9 - error: Cannot access member "gui" for type "MyDataclass"
    Member "gui" is unknown (reportGeneralTypeIssues)
1 error, 0 warnings, 0 informations 

Obviously the type checker can't know about the gui field that gets added to the class.

If you are interested in suggestions, dataclass_transform can be used on a class not just a function, which allows you to create a superclass that has a gui property or field but also acts like a dataclass:

from magicgui.experimental import GuiClass

class MyDataclass(GuiClass):
    ...

multimeric avatar Feb 23 '25 14:02 multimeric

Thanks @multimeric, yeah indeed, python doesn't support intersection types (https://github.com/python/typing/issues/213) which is what we'd really need here.

We do actually already have the class-based approach you are describing here, it's just not public:

Image

However, since all this class really does is add two descriptors, I think I'd rather just make that descriptor public and document it's usage. For what it's worth, the pattern here is very similar to what we do in psygnal with the @evented descriptor (which is also used by guiclass). See relevant psygnal docs here. I thought I had documented this pattern in the magicgui docs, but it looks like I didn't. What I think we should do here is just make the GuiBuilder descriptor public here, and then encourage a usage like this:

from dataclasses import dataclass
from typing import ClassVar, reveal_type

from psygnal import SignalGroupDescriptor
from magicgui.schema._guiclass import GuiBuilder


@dataclass  # or any other dataclass-like structure
class MyDataclass:
    x: int = 42

    events: ClassVar[SignalGroupDescriptor] = SignalGroupDescriptor()
    gui: ClassVar[GuiBuilder] = GuiBuilder()

That avoids all the dynamic attribute adding, and makes much more explicit what's happening there. Also makes it easier for people to pick their own names. The only "catch" at the moment is that the name 'events' is a hardcoded assumption in GuiBuilder... but that would be a couple line change. So... I think that would be my preferred typing friendly approach (it's what I use when using this)

tlambert03 avatar Feb 23 '25 17:02 tlambert03

ah actually, I forgot I did most of the work in making GuiBuilder public and agnostic to the name "events" in https://github.com/pyapp-kit/magicgui/pull/688/ will try to get that merged soon and update the docs

tlambert03 avatar Feb 23 '25 18:02 tlambert03