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

Experiment: Add a GUI-providing plugin #507

Closed
wants to merge 1 commit into from
Closed

Conversation

corranwebster
Copy link
Contributor

This is a proof-of-concept which follows from some discussions about disentangling the GUI and SplashScreen objects: when using the SplashScreen support in a GUI object it requires very early creation of the GUI object, when it is really only needed at during the run method.

This shows how once this separation is done, the GUI object can be supplied by a plugin, and demonstrates use with the GUIApplication and TasksApplication classes and corresponding examples.

This attempts to be backwards compatible with existing code that (a) supplies a GUI directly and/or SplashScreen directly, or (b) relies on a default GUI and also doesn't know about GUIPlugin.

No tests right now, although existing tests might pass (it would be good if they did).

@corranwebster corranwebster changed the title Add a GUI-providing plugin Experiment: Add a GUI-providing plugin Mar 9, 2023
@mdickinson
Copy link
Member

mdickinson commented Mar 23, 2023

@corranwebster and I have discussed this a bit offline. I'm summarising some of that discussion here (so the below isn't for @corranwebster, but for other readers, and possibly also for future me and Corran).

  • I really like the general idea, and this seems like the right way forward for some of the problems we're facing downstream. In particular, we have downstream code that needs a different IGUI implementation, but requires users to modify their main application scripts to instantiate their TasksApplication subclass in a special way to use that IGUI implementation; it would be quite convenient if all they had to do was use the appropriate plugin instead.

  • While this approach works (and has been made to work downstream), it feels to me as though IGUI is sufficiently far from the "right thing" at this point that I'd be reluctant to lock that choice in here. It seems worth exploring the idea of an IEventLoop in Pyface. This would look quite a lot like a subset of IGUI - there would be methods to start and stop the event loop, and to contribute callbacks - possibly from a background thread - to the event loop for eventual processing on that event loop, but unrelated parts of IGUI, like the splash screen, or the application icon, wouldn't be included.

@mdickinson
Copy link
Member

  • This would look quite a lot like a subset of IGUI

In particular, this would allow initial "cheap" solutions where an existing GUI subclass could simply be declared as implementing IEventLoop, or where we have an event loop that just delegates to a GUI instance.

@corranwebster
Copy link
Contributor Author

Thanks for the feedback.

I guess to keep consistency if we had IEventLoop as a new superinterface of IGUI it might look something like:

class IEventLoop(Interface):

    def create(self):
         # after calling create, can start and stop and register things to be invoked

    def destroy(self):
        # after calling destroy, can no longer start, stop or invoke

    def start_event_loop(self):
        ...
 
    def stop_event_loop(self):
        ...

    def invoke_later(self, callback, ...):
        ...

    def invoke_after(self, delay, callback, ...):
        ...

etc.

I think we either have to have invoke_after, a common timer API, or some sort of "stop event loop after x seconds" timeout function, because it's basic functionality needed for timeouts in tests (ie. you need to be able to do something like invoke_after(10, event_loop.stop_event_loop). It might not need to be part of the core API and instead in a test assistant, though.

@mdickinson
Copy link
Member

it might look something like [...]

Yep, that's pretty much what I was imagining. Agreed about needing something that does timing.

There's also things like set_trait_later and set_trait_after; those could possibly be provided by some kind of MEventLoop mixin, since they're only really special cases of invoke_later and invoke_after.

@mdickinson
Copy link
Member

@corranwebster I've recreated this PR as #562, since the style fixes in main made a direct merge of main into this branch very messy. If you prefer, I can do a force push on this branch instead. Preferences?

@corranwebster
Copy link
Contributor Author

Let's close this one.

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

Successfully merging this pull request may close these issues.

2 participants