-
Notifications
You must be signed in to change notification settings - Fork 3
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
MNT: Common table views, Primarily LivePVTableModel, minor skeleton work on NestableTableModel #72
Conversation
81dba3f
to
c3eff7f
Compare
I may work on #71 first... these tables want those data types, and I'll need to rebase this branch anyway |
c3eff7f
to
b353e4e
Compare
This got too long, so I'm cutting it off at the |
…work on LivePVTableModel and NestableTableModel
…nd refactor LivePVTableModel.is_close
…tem helper method
9576c77
to
9e6b857
Compare
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 think this was all nitpicks and questions, there isn't anything bad here
Feel free to discard the ones you disagree with
_bridge_cache: ClassVar[ | ||
WeakValueDictionary[int, QDataclassBridge] | ||
] = WeakValueDictionary() | ||
bridge: QDataclassBridge |
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.
What is the data class bridge used for with EntryItem
? I remember its usage in atef but I don't see any other references to bridge
here.
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.
We haven't used this in superscore yet, but its usage is intended to be similar to atef
. At some point we'll probably update an entry and want to make sure that updates in the tree view.
superscore/widgets/views.py
Outdated
self.client = client | ||
self.open_page_slot = open_page_slot | ||
self.poll_rate = poll_rate | ||
self._data_cache = {e.pv_name: None for e in self.entries} # noqa: F821 |
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.
Is None
preferable here to some sort of disconnected/not connected yet EpicsData
value?
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.
This wasn't entirely intentional, None
was just the easiest for me to initialize at the time. It might helpful to be able to differentiate between the not-yet-attempted state and an attempted-but-failed state, but I'm not wedded to it.
I suppose it might be better to keep the types consistent 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.
LGTM
Description
Build a couple table models that can hopefully be reused elsewhere. Includes:
LivePVTableModel
: a model for displaying {Parameter, Readback, Setpoint} data alongside the current valuesNestableTableModel
: a model for {Collection, Snapshot} entries, which shows title / description primarilyBaseTableModel
that holds their own entriesAlso:
_PVPollThread
for grabbing data outside the GUI threadNote: I renamed
superscore.widgets.tree
->superscore.widgets.views
, but the diff viewer doesn't pick that up. I did not change the tree model at all, so the reviews can focus on the table modelsTo-do (in a later PR at this point):
Motivation and Context
Building screens, I fell down this rabbit hole and the diff warranted a separate PR
How Has This Been Tested?
Interactively so far, a couple tests added
Where Has This Been Documented?
This PR
Pre-merge checklist
docs/pre-release-notes.sh
and created a pre-release documentation page