Refactor SF plotter #2592
Refactor SF plotter #2592
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the sweep formula plotter by extracting plotting logic from the monolithic SF_FormulaPlotter function into two new helper functions: SF_CreateTracesForResultsImpl (which handles the actual trace creation for individual results) and SF_CreateTracesForResults (which orchestrates the trace creation for a set of formula results). This refactoring aims to improve code organization and maintainability while maintaining PSX compatibility.
Key changes:
- Extracted ~220 lines of plotting logic into new function
SF_CreateTracesForResultsImpl - Created intermediary function
SF_CreateTracesForResultsto coordinate between the plotter and implementation - Modified annotation and table formula tracking to use wave note indices instead of separate counter variables
- Added
/Zflag to a test wave reference that's expected to be null
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Packages/tests/Basic/UTF_SweepFormula.ipf | Adds /Z flag to wave reference expected to be null, preventing errors |
| Packages/MIES/MIES_SweepFormula.ipf | Major refactoring extracting plotting logic into new helper functions and updating state tracking mechanism |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MichaelHuth
force-pushed
the
feature/2592-refactor_sf_plotter2
branch
from
c8ba37e to
ced4ebe
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MichaelHuth
force-pushed
the
feature/2592-refactor_sf_plotter2
branch
from
c15cefd to
08d2acc
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MichaelHuth
force-pushed
the
feature/2592-refactor_sf_plotter2
branch
2 times, most recently
from
47d9f04 to
6f5681f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MichaelHuth
force-pushed
the
feature/2592-refactor_sf_plotter2
branch
from
6f5681f to
73f1fdc
Compare
this will allow storing it indexed
…lay type This allows to build graph and table window waves independently
Before the display windows (table/graph) were created before the formulas were evaluated, now after each formula was evaluated a window is created and filled with traces. The window naming was unified to use "graph"/"table" suffixes for the base as well as for the subwindows. This change allows to react on future operation results with full plotting content.
no functional change
no functional change
no functional change
no functional change
…indows - moved SF constants for window suffix and display type to global constants - extended descriptive code comment for SFH_GetBrowserForFormulaGraph - renamed SFH_GetFormulaGraphForBrowser to SFH_GetFormulaPanelFromBrowser as it never returned a graph window (it returned the panel) - extended SFH_GetFormulaPanelFromBrowser to work also for panels containing table subwindows - renamed SFH_GetFormulaGraphs to SFH_GetFormulaPlotPanels to reflect actual behavior and extended the functions code documentation
MichaelHuth
force-pushed
the
feature/2592-refactor_sf_plotter2
branch
from
73f1fdc to
978cd00
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@MichaelHuth, I tested the plotting with PSX; everything looks good. |
This PR will replace #2586. The approach implemented in #2586 is not compatible with PSX.
This PR moves the plotter components into separate functions.
It revises the naming of the plotters panel and subWindows for graphs and tables to follow a common concept. The creation of plot windows is moved inside the plotting loop (before it was done for all windows first), such that after each formula evaluation a new output window is created and reused until the next
andis encountered.This paves the way the add support for operations (like ivscc_apfrequency) that will include a full plotting setup in the output. This needs to be inserted then in the plotting loop.
This PR also fixes some potential bugs that were discovered:
table(x)and regular expressions within the samewithblock properly when there were multiple formulas contributing to the same table and graph subwindow.formulasAreDifferentwas not reset in between new graph windows, such that under specific circumstances trace style properties were carried over to the next graph subwindow