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

Better type checking integration for my_gui_class.gui #691

Open
multimeric opened this issue Feb 23, 2025 · 2 comments
Open

Better type checking integration for my_gui_class.gui #691

multimeric opened this issue Feb 23, 2025 · 2 comments

Comments

@multimeric
Copy link

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):
    ...
@tlambert03
Copy link
Member

Thanks @multimeric, yeah indeed, python doesn't support intersection types (python/typing#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
Copy link
Member

ah actually, I forgot I did most of the work in making GuiBuilder public and agnostic to the name "events" in #688 will try to get that merged soon and update the docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants