-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add support for writing to user-editable variables through the API #284
base: master
Are you sure you want to change the base?
Conversation
damnit/gui/main_window.py
Outdated
@@ -944,6 +945,42 @@ def __init__(self, file_path: Path, parent=None): | |||
self.setCentralWidget(self.text_edit) | |||
self.resize(1000, 800) | |||
|
|||
class QtUpdateAgent(UpdateAgent, QtCore.QObject): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate this isn't really part of this PR, but is there any reason we need the update sender & update receiver to be one class? It seems like all they share is the topic name, so they could easily be independent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing is better than two? 😛 I dunno, either design seems fine to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's an easy way to avoid multiple inheritance, I generally think it's worth taking. I don't feel strongly enough to hold this up, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, that makes sense. I refactored it in 6ca1363.
LGTM! |
So that we can use it in the API, where we don't want to depend on Qt. Also removed the `description` argument from `variable_set()` because it's currently unused.
Thanks James, LEBTM (Looks Even Better To Me)! |
Thanks :) I found a bug to do with writing to the |
This has been requested by various users. Note that currently we only support writes for user-editable variables because writing to the HDF5 files would require some more coordination.