Refactored Device wrapping #37
Conversation
The multi-threading tests no longer work and cause deadlock, which is to be expected since the thread patching is gone. A simpler solution will be implemented later.
|
Following up on the discussion, some reference on how you could translate the current ExEngine device APIs into Protocols can be found from bluesky: https://github.com/bluesky/bluesky/blob/main/src/bluesky/protocols.py I would personally love to switch to a structural typing approach because it's much more versatile than using abstract classes and it would greatly simplify my project implementation as well. |
Makes sense
I agree this way of doing it makes much better sense, and I'd support removing the inheritance route and making wrapping the only way of using it. If we wanted to make classes that automatically come wrapped when constructed, easy enough to do it in a |
| self._finished = False | ||
| self._initialized = False | ||
| # Check for method-level preferred thread name first, then class-level | ||
| self._thread_name = getattr(self.execute, '_thread_name', None) or getattr(self.__class__, '_thread_name', None) | ||
|
|
||
| def __lt__(self, other) -> bool: |
There was a problem hiding this comment.
Are you thinking to make the events in a priority queue? So you could submit an event that gets jumped ahead of existing ones? I had a mechanism for this that perhaps you saw in executor.submit, but I think this makes more sense
| @@ -0,0 +1,64 @@ | |||
| import queue | |||
There was a problem hiding this comment.
It might be good to give this a more descriptive filename?
|
Test setup is failing. I think its because you need to do a relative import here: File "D:\a\ExEngine\ExEngine\src\exengine\kernel\queue.py", line 6, in <module>
from exengine.kernel.ex_event_base import ExecutorEvent
ModuleNotFoundError: No module named 'exengine'
[end of output]I think you need |
|
I made some more small comments, but looking really good so far! One issue I've thought about is what the user facing API for a wrapped object should look like: is any call to |
A completely refactored Device wrapping mechanism, that can wrap openwfs and arbitrary Python objects.
Main changes:
ExEngine is no longer a singleton class. Each device explictly takes an ExEngine as input to the constructor. Rationale: the singleton antipattern is problematic for testing and scalability. For example: imagine that a client uses two two packages that both use ExEngine. The first package may shut down its own ExEngine at some point. Of course, this should not shut down the other ExEngine. By dropping the singleton pattern this problem is avoided. Moreover, mocking the ExEngine is simpler and it is clear which parts of the code access the engine and which parts do not.
Device base class and wrapping. ExEngine provides a wrapper that makes attribute access thread safe, converts method calls to delayed execution and (not implemented yet) adds metadata, notifications, etc. To wrap an existing object, call
engine.register, and make sure never to use the bare (non-wrapped) object. All internal attribute access inside the object itself is unaffected by the wrapper (so no need to monkey patch the threading library). Alternatively, an object can derive from the Device base class. This base class no longer does much, except that it overrides__new__to callengine.registerautomatically. Examples are included in test_preferred_thread_annotations.py and test_generic_device.pyThe option to shutdown queues was implemented through condition variables and complicated the code. In Python 3.13, this functionality is natively present in the Queue and PriorityQueue objects. The new code uses these objects when available, and provides a compatible implementation for Python versions < 3.13
Open issues:
get_property_limitsetc.