enthought / pyface

pyface: traits-capable windowing framework
Other
105 stars 55 forks source link

Feature Request: add 'close' method to IGUI interface #1200

Open mdickinson opened 1 year ago

mdickinson commented 1 year ago

Some downstream IGUI implementations allocate system resources, or modify global state, at instance creation time. (For example, one GUI subclass that we have in a downstream project changes the asyncio global event loop.) As a result, we've added close methods to those implementations to allow those resources to be released, and global state changes to be reverted. It's particularly important to have this functionality available in test suites, where many IGUI instances are being created and destroyed within a single process.

To allow polymorphic cleanup, it would be useful to add a close method to the IGUI interface itself, and the GUI base class. The base class implementation would do nothing.

This isn't without backwards compatibility risks: as always, an interface is a point of rigidity in a system, and in particular adding a close method would invalidate any existing implementations of IGUI that don't already inherit from theGUI base class. However, I'm not aware of any such implementations in practice, and I think the risk here is manageable.

On details: we'd need to decide whether it's also useful to have an is_closed property. The minimal useful interface would be just the close method, together with a guarantee that close is idempotent, so that it's always safe to call close even if you're not sure whether the IGUI object is already closed.

corranwebster commented 1 year ago

This seems reasonable, although it may be worth matching the create/destroy naming of the IWidget interface for consistency, although that would be a more invasive change to shift any initialization code out of __init__ and into a create method, so I'm not 100% sure of this. However the fact that this is to deal with global state makes me think that a side-effect free __init__ may be beneficial.

In terms of semantics, what should be the behaviour after close() is called? Which methods, if any, should be expected to raise?

In terms of an is_closed attribute, I'm not sure that I would expose anything publicly, and I suspect any internal use may be able to be handled by things like setting application objects or other global state to None and using that as the flag.

mdickinson commented 1 year ago

In terms of semantics, what should be the behaviour after close() is called? Which methods, if any, should be expected to raise?

Most of them, I'd expect - the gui object would be unusable after closing. In the case of interest, we have an associated asyncio event loop and the GUI subclass close method closes that event loop, after which it's not useful.

corranwebster commented 1 year ago

An idea that's been percolating for a few days around this: if we add start/stop methods then they could explicitly set Traits "ui" dispatch (or a better architected replacement) to use their call_later on start and then restore it on stop.

Probably can't do it with the current GUI object, unfortunately, as the usage is too chaotic (because they explicitly aren't singletons but give access to the QApplication singleton, we tend to create them and throw them away or even just use the class); we might need to be a new IEventLoop or something which the GUI objects reference to do their stuff.