feat: Enable saving/loading of math-derived signals in plotlists

[miker2]

This change introduces the capability to save and load plotlists that include signals derived from various math operations, ensuring that the full state of the plotted signals can be reproduced.

Key changes include:

1. PlotSpec Class: I created a new PlotSpec class (plot_spec.py) to encapsulate all necessary information to define and reproduce a signal. This includes its name, source (file or math operation), unique ID, and derivation details (expression, operation parameters, input signals' PlotSpecs). It includes to_dict and from_dict methods for serialization.

2. MathsWidget Modifications:

   • VarInfo and DataItem now store associated PlotSpec objects.
   • Math operations (expressions, filters, differentiation, etc.) now generate a PlotSpec for their output signals, capturing the operation type, parameters, and input signal PlotSpecs.
   • MathsWidget can use inputs from both var_in (file signals) and var_out (derived signals) for new operations.
   • A new method execute_operation_from_spec allows MathsWidget to re-calculate a signal based on its PlotSpec and provided input DataItems during plotlist loading.

3. CustomPlotItem Update: Now stores a PlotSpec for each trace. If a signal is plotted directly from a file, a basic "file" PlotSpec is created. If plotted from MathsWidget, it uses the detailed PlotSpec attached to the DataItem.

4. Plot Saving/Loading Logic:

   • When saving, PlotSpec objects are converted to dictionaries via to_dict() and stored in the plotlist file.
   • When loading, PlotAreaWidget.generate_plots uses a new helper _reproduce_signal. This function:
     • Converts dictionaries back to PlotSpec objects.
     • Handles "file" type signals by fetching them from the DataFileWidget.
     • Handles "math" type signals by recursively reproducing input signals and then calling MathsWidget.execute_operation_from_spec to re-compute the derived signal.
     • Uses a cache to avoid re-computing the same signal multiple times if it's an input to several other signals.

5. Supporting Changes:

   • DataModel and ThinModelMock were updated to ensure DataItem objects (with their PlotSpecs) are correctly retrieved.
   • SubPlotWidget.plot_data_from_source was adapted to handle plotting of these reproduced DataItems.
   • DataFileWidget now has a method to look up DataItems by a file identifier and signal name.

This enables you to save complex analysis sessions involving multiple math-derived signals and reliably restore them later.

[gemini-code-assist]

Hello @miker2, I'm Gemini Code Assist¹! 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!

Summary of Changes

Hello team,

Gemini here, providing a summary of this pull request authored by @miker2.

This PR introduces a significant enhancement to the application's plotting capabilities, specifically enabling the saving and loading of plotlists that include signals derived from mathematical operations performed within the MathsWidget. Previously, only signals loaded directly from files could be reliably saved and restored in plot configurations. This change addresses that limitation by implementing a mechanism to capture the 'recipe' for derived signals.

The core of this feature is the introduction of a PlotSpec object, which encapsulates all the necessary information to uniquely identify and reproduce a signal, whether it originates from a file or is the result of a mathematical transformation. This PlotSpec is now associated with DataItems and CustomPlotItems. The saving process serializes these PlotSpecs, and the loading process uses them to either retrieve the original file data or re-execute the necessary math operations, including recursively reproducing any input signals required for a derived signal.

Highlights

• New PlotSpec Class: A new class PlotSpec is introduced (plot_spec.py) to store metadata about a signal, including its source type ('file' or various 'math' types), unique ID, file details (if applicable), and math operation details (expression, parameters, input PlotSpecs) if it's a derived signal. It includes to_dict and from_dict methods for serialization.
• DataItem and VarInfo Updates: The DataItem class (data_model.py) and the internal VarInfo dataclass (maths_widget.py) are updated to include a plot_spec attribute to store the associated PlotSpec object.
• MathsWidget Integration: The MathsWidget (maths_widget.py) is modified to generate and attach PlotSpec objects to the DataItems it produces from mathematical operations (expressions, filters, diff/int, running window/minmax). It also gains a new method execute_operation_from_spec to re-compute a signal based on a provided PlotSpec and its inputs during loading.
• CustomPlotItem Enhancement: The CustomPlotItem (custom_plot_item.py) now stores the PlotSpec for the trace it represents. When a signal is plotted, the PlotSpec is either provided (for derived signals) or a default 'file' PlotSpec is created.
• Plot Saving/Loading Logic: The plot saving mechanism (sub_plot_widget.py) is updated to serialize the PlotSpec of each trace to a dictionary. The loading logic (plot_manager.py) introduces a recursive helper function _reproduce_signal that uses the saved PlotSpec dictionaries to either fetch file-based signals via DataFileWidget or re-calculate math-derived signals using the new MathsWidget.execute_operation_from_spec method, utilizing a cache to avoid redundant computations.
• DataFileWidget Lookup: The DataFileWidget (data_file_widget.py) is updated to store references to its VarListWidget instances and includes a new method get_data_item_by_file_id_and_name to facilitate retrieving original DataItems based on the file identifier stored in the PlotSpec.

Changelog

Click here to see the changelog

• custom_plot_item.py
  • Import PlotSpec and TYPE_CHECKING (lines 17-23)
  • Add plot_spec_from_source parameter to __init__ (line 29)
  • Initialize self.plot_spec, creating a default 'file' PlotSpec if plot_spec_from_source is None (lines 74-97)
  • Update get_plot_spec to return the stored self.plot_spec object (line 157)
• data_file_widget.py
  • Add self.var_lists list to track VarListWidget instances (line 47)
  • Store VarListWidget instance directly in self.sources and add it to self.var_lists when opening a file (lines 76-78)
  • Update close_file to remove the closed widget from self.var_lists and self.sources, and reset slider limits if no files remain (lines 94-105)
  • Modify get_data_file_by_name to use .get() for safer access (line 121)
  • Add get_data_item_by_file_id_and_name method to retrieve DataItems from loaded files using file identifier and signal name (lines 123-157)
  • Add validity check for idx in get_time (lines 166-169)
• data_model.py
  • Import PlotSpec (line 7)
  • Add plot_spec parameter to DataItem.__init__ and store it (line 15-18)
  • Add plot_spec property and setter to DataItem (lines 33-39)
  • Update get_data_by_name to return the DataItem object and ensure a 'file' PlotSpec is created if missing for file-loaded items (lines 113-138)
• maths/diff_int.py
  • Add get_operation_details and get_source_type methods to DifferentiateSpec (lines 22-27)
  • Add get_operation_details and get_source_type methods to IntegrateSpec (lines 46-51)
• maths/filter.py
  • Add get_operation_details and get_source_type methods to FilterSpec (lines 87-98)
• maths/maths_base.py
  • Import PlotSpec (line 10)
  • Modify eventFilter to create and assign a PlotSpec to the output DataItem for math operations, capturing input specs and operation details (lines 81-112)
  • Update add_new_var signature to require and store the plot_spec (line 112, 115)
• maths/running_minmax.py
  • Add get_operation_details and get_source_type methods to RunningMinMaxSpec (lines 113-123)
• maths/running_window.py
  • Import WindowTypes (line 19)
  • Add get_operation_details and get_source_type methods to RunningWindowSpec (lines 114-124)
• maths_widget.py
  • Import PlotSpec, WindowTypes, MinMaxType (lines 19-24)
  • Add plot_spec attribute to VarInfo dataclass (line 63)
  • Modify dropEvent to create and store a 'file' PlotSpec for dropped items in VarInfo (lines 195-205)
  • Modify evaluate_math to create and assign a 'math_expr' PlotSpec to the output DataItem and pass it to add_new_var (lines 260-295)
  • Update add_new_var signature to require and store the plot_spec in VarInfo (line 297, 305)
  • Modify start_drag to ensure the DataItem being pickled has its PlotSpec set from the VarInfo (lines 317-327)
  • Add execute_operation_from_spec method to reproduce math operations based on a PlotSpec and input DataItems (lines 381-510)
• plot_manager.py
  • Import PlotSpec, DataItem, TYPE_CHECKING, Dict, Any, MathsWidget, DataFileWidget (lines 9-15)
  • Add _reproduce_signal recursive helper method to reconstruct DataItems from PlotSpec dictionaries, handling 'file' and 'math' sources and using a cache (lines 309-414)
  • Modify generate_plots to use _reproduce_signal to obtain DataItems for plotting, passing necessary widget references and a cache (lines 440-496)
  • Update generate_plots to pass the DataItem, a source reference, and a flag to sub_plot_widget.plot_data_from_source (lines 491-494)
  • Update generate_plots to use plot_dict variable name consistently (lines 451, 499)
• plot_spec.py
  • New file defining the PlotSpec class with attributes for signal metadata and to_dict/from_dict methods (lines 1-46)
• sub_plot_widget.py
  • Import PlotSpec, DataItem (lines 9-10)
  • Modify plot_data_from_source signature to accept name_or_data_item, source_or_none, and is_reproduced_item flag (line 125)
  • Update plot_data_from_source logic to handle plotting either from a name/source pair (original) or a DataItem (reproduced), ensuring PlotSpec, data, and time are correctly obtained (lines 133-183)
  • Pass the obtained PlotSpec to the CustomPlotItem constructor (lines 194-196)
  • Update onClose signal connection logic to handle the potentially different source_for_custom_item and add error handling/warnings (lines 201-207)
  • Update get_plot_info to call .to_dict() on the PlotSpec objects before adding them to the plot info dictionary (line 257)

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 is currently in preview and 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 to provide feedback.

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.

---

A signal derived,
Its recipe saved,
Through math it's revived,
On the plot, it's engraved.

Footnotes

1. 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. ↩

[gemini-code-assist]

Code Review

This pull request introduces a very valuable feature for saving and loading plotlists, especially those with math-derived signals. The new PlotSpec class and the associated logic for signal reproduction are well-designed and address a complex problem. The changes are extensive and show a good understanding of the application's architecture.

I've identified a few areas for potential improvement and one likely bug. Overall, this is a strong contribution.

Summary of Findings

• Potential Bug in Plot Y-Range Setting: In plot_manager.py, generate_plots method, there's a reference to plot["yrange"] after plot was likely renamed to plot_dict. This could lead to a NameError.
• Ad-hoc Attribute Assignment: The dynamic assignment of idx to VarListWidget instances in data_file_widget.py could be made more explicit by defining idx in VarListWidget's __init__.
• Placeholder Identifiers: Fallback string identifiers like "unknown_source_widget" are used. Consider defining these as shared constants if their usage grows.
• Robustness of Math Operation Reproduction: The execute_operation_from_spec in maths_widget.py is complex. While it appears to correctly reimplement operations, continued testing for edge cases is important. Error handling via print could be enhanced with custom exceptions or logging.
• Clarity of Refactored Methods: Methods like SubPlotWidget.plot_data_from_source have become more complex. Adding docstrings would improve maintainability.

Merge Readiness

This pull request introduces a significant and valuable feature. The implementation is largely robust and well-thought-out.

Before merging, I recommend addressing the potential NameError in plot_manager.py (marked as 'high' severity). The 'medium' severity comments are suggestions for improving maintainability and robustness, and can be addressed now or in follow-up work at the author's discretion.

As a reviewer, I am not authorized to approve pull requests. Please ensure further review and approval from authorized maintainers before merging.

[gemini-code-assist]

It seems plot was renamed to plot_dict on line 451, but here plot["yrange"] is still used. Should this be plot_dict["yrange"] to avoid a NameError?

subplot.set_y_range(*plot_dict["yrange"])

[gemini-code-assist]

The use of placeholder strings like "unknown_source_widget" for file_id when a more specific identifier can't be found is a reasonable fallback. However, if these kinds of identifiers become more common or need to be managed more strictly across different parts of the application (e.g., here and in DataModel), would it be worth defining them as constants in a shared module to ensure consistency and avoid typos?

[gemini-code-assist]

Assigning var_list.idx = tab_idx dynamically adds an attribute to VarListWidget instances. While this works for PlotSpec compatibility, it can make the VarListWidget class contract less explicit. If idx is a consistently needed piece of information for VarListWidget, would it be clearer to formally add it as an optional parameter in its __init__ method and store it as a regular instance attribute?

[gemini-code-assist]

The value 1.e9 is used here to reset the slider limits when no files are open. If this large default maximum is used in other places or has a specific meaning (e.g., 'effectively infinite for practical purposes'), could defining it as a named constant (e.g., DEFAULT_SLIDER_MAX_RANGE) improve readability and maintainability?

[gemini-code-assist]

The fallback file_id = "unknown_data_model_source" is noted. The comment on lines 124-128 about potentially enhancing DataModel to store the filename (perhaps from data_loader.source during initialization) is a good point. This would make the file_source_identifier more robust here. For now, this fallback is understandable, but it's a good candidate for future refinement.

[gemini-code-assist]

The ThinModelMock.get_data_by_name method and the TODO comment on line 350 acknowledge this as a hack. While this PR focuses on PlotSpec, this area remains a point of technical debt. Is there a plan to refactor this as part of unifying the plotting mechanisms, as the comment suggests? This current implementation, while functional for the drag/drop, could be a source of confusion or bugs later.

[gemini-code-assist]

This execute_operation_from_spec method is critical for reproducing signals and is quite comprehensive. A few points:

1. Error Handling: Currently, errors are reported using print(). For more robust error management, especially if this component were part of a larger system or library, would it be beneficial to raise specific exceptions that calling code could catch and handle more gracefully (e.g., MathOperationError, MissingParameterError)?
2. Enum String Comparison: Comparisons like op_details['type'] == MinMaxType.MIN.name.lower() (line 468) and WindowTypes.MEAN.name.lower() (line 488) rely on the string representation of enum names. This is consistent with how get_operation_details seems to store them. It's generally okay but ensure that the string format (.name.lower()) is consistently used when these details are generated and consumed.
3. Code Duplication in Imports: from scipy import signal as scipy_signal is imported twice locally (lines 424, 474). If scipy.signal usage expands or for general cleanliness, a single import at the top of the file or class level might be preferred, though local imports are fine for limiting scope.

[gemini-code-assist]

The logic to match file_source_identifier by checking vl.filename and then str(vl.idx) is a good fallback mechanism. The comment in DataFileWidget.get_data_item_by_file_id_and_name about this being less robust if tab order changes is pertinent here too. This seems like the best approach given the current structure.

[gemini-code-assist]

The PlotSpec class is well-designed for its purpose, providing clear serialization (to_dict) and deserialization (from_dict) methods. The use of uuid.uuid4() for unique_id is a good choice for ensuring unique identification of signal specifications. The handling of optional fields with .get() in from_dict is also robust.

[gemini-code-assist]

The refactoring of plot_data_from_source to handle both new plots and reproduced DataItem objects is a significant improvement.

• The logic to differentiate behavior based on is_reproduced_item is clear.
• Passing plot_spec_for_custom_item to CustomPlotItem is crucial.
• The handling of source_for_custom_item and its onClose signal connection (lines 201-207) with a try-except block and warnings is a good defensive measure.

Consider adding a docstring to this complex method to explain the roles of name_or_data_item, source_or_none, and is_reproduced_item for future maintainers.