feat: Display variable values in VarListWidget #16
Conversation
This commit introduces a feature to display the values of each variable in the `VarListWidget` corresponding to the current time, as indicated by the plot cursor. - A "Show values" checkbox has been added to the `DataFileWidget` to toggle this functionality. - The `DataModel` has been extended to track the current tick and whether to display values. The `data()` method now formats the display string to include the variable's value when the feature is enabled. - Signal/slot connections have been added to propagate the cursor's tick from the `PlotManager` to the `DataModel` and to update the view when the cursor moves or the checkbox state changes.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @miker2, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a significant new feature that allows users to view the real-time values of variables within the VarListWidget, synchronized with the plot cursor's position. This is achieved by adding a dedicated "Show values" checkbox in the DataFileWidget for user control. Under the hood, the DataModel has been enhanced to track the current plot tick and the display preference, dynamically updating how variable names are presented to include their corresponding values. This functionality is seamlessly integrated through new signal and slot connections, ensuring that the variable list updates automatically as the plot cursor moves or the display option is toggled.
Highlights
- User Interface Enhancement: A new "Show values" checkbox has been integrated into the
DataFileWidget's user interface, allowing users to toggle the display of variable values. - Data Model Logic: The
DataModelhas been extended to maintain the current plot cursor tick and a boolean flag indicating whether variable values should be displayed. Thedata()method now dynamically formats the display string to include the variable's value at the current tick when the feature is active. - Inter-Component Communication: New signal/slot connections have been established to ensure that changes in the plot cursor's position (tick) are propagated from the
PlotManagerto theDataFileWidgetand subsequently to theDataModel. Additionally, the "Show values" checkbox state is connected to update theDataModeland refresh theVarListWidgetview.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request successfully introduces a feature to display variable values in the VarListWidget, toggled by a new checkbox. The implementation correctly uses Qt's signal/slot mechanism to propagate state changes. My review includes a couple of suggestions to align with Qt's model/view best practices for better performance and to improve code readability in the data model.
| try: | ||
| value = item.data[self._current_tick] | ||
| if isinstance(value, (float, np.floating)): | ||
| display_text = f"{var_name}: {value:.4f}" | ||
| else: | ||
| display_text = f"{var_name}: {value}" | ||
| return QVariant(display_text) | ||
| except IndexError: | ||
| return QVariant(f"{var_name}: N/A") |
There was a problem hiding this comment.
The logic for creating the display text can be refactored to improve readability and reduce repetition. By preparing the value as a string first and then constructing the final display text in one place, you can avoid repeating the f"{var_name}: ..." pattern and have a single return statement for this part of the logic.
| try: | |
| value = item.data[self._current_tick] | |
| if isinstance(value, (float, np.floating)): | |
| display_text = f"{var_name}: {value:.4f}" | |
| else: | |
| display_text = f"{var_name}: {value}" | |
| return QVariant(display_text) | |
| except IndexError: | |
| return QVariant(f"{var_name}: N/A") | |
| try: | |
| value = item.data[self._current_tick] | |
| if isinstance(value, (float, np.floating)): | |
| value_str = f"{value:.4f}" | |
| else: | |
| value_str = f"{value}" | |
| display_text = f"{var_name}: {value_str}" | |
| except IndexError: | |
| display_text = f"{var_name}: N/A" | |
| return QVariant(display_text) |
| def set_show_values(self, show): | ||
| self.model().set_show_values(show) | ||
| self.model().layoutChanged.emit() | ||
|
|
||
| def update_tick(self, tick): | ||
| self.model().set_current_tick(tick) | ||
| self.model().layoutChanged.emit() |
There was a problem hiding this comment.
For better performance and adherence to Qt's model/view architecture, it's preferable to use dataChanged instead of layoutChanged when only the data for items is changing, not the structure of the model (like adding or removing rows).
dataChanged is more specific and can be more efficient as it tells the view to only update the data of existing items. layoutChanged is a stronger signal that may cause the view to perform more work than necessary, such as recomputing layout hints.
| def set_show_values(self, show): | |
| self.model().set_show_values(show) | |
| self.model().layoutChanged.emit() | |
| def update_tick(self, tick): | |
| self.model().set_current_tick(tick) | |
| self.model().layoutChanged.emit() | |
| def set_show_values(self, show): | |
| self.model().set_show_values(show) | |
| if self.model().rowCount() > 0: | |
| self.model().dataChanged.emit(self.model().index(0), self.model().index(self.model().rowCount() - 1)) | |
| def update_tick(self, tick): | |
| self.model().set_current_tick(tick) | |
| if self.model().rowCount() > 0: | |
| self.model().dataChanged.emit(self.model().index(0), self.model().index(self.model().rowCount() - 1)) |
This commit introduces a feature to display the values of each variable in the
VarListWidgetcorresponding to the current time, as indicated by the plot cursor.DataFileWidgetto toggle this functionality.DataModelhas been extended to track the current tick and whether to display values. Thedata()method now formats the display string to include the variable's value when the feature is enabled.PlotManagerto theDataModeland to update the view when the cursor moves or the checkbox state changes.