Skip to content

SF plotter use structure to aggregate argument data, add support for full plotting spec#2630

Merged
t-b merged 15 commits intomainfrom
feature/2630-use_struct_in_sf_plotter
Feb 24, 2026
Merged

SF plotter use structure to aggregate argument data, add support for full plotting spec#2630
t-b merged 15 commits intomainfrom
feature/2630-use_struct_in_sf_plotter

Conversation

@MichaelHuth
Copy link
Collaborator

@MichaelHuth MichaelHuth commented Feb 9, 2026

This is a preparation for PR #2599 where the first part of #2599 was split off to this PR

  • adds a structure for plotter function arguments to reduce number of arguments for sub functions

  • refactor a few more code pieces out to separate functions

  • add support for full plotting specification in plotter main loop

  • add tests

close #2626

@MichaelHuth MichaelHuth self-assigned this Feb 9, 2026
Copilot AI review requested due to automatic review settings February 9, 2026 17:35
@MichaelHuth MichaelHuth requested a review from t-b as a code owner February 9, 2026 17:35
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the SweepFormula plotter to reduce argument threading by introducing a plotter state structure, extracting some logic into helper functions, and adding support for a “full plotting specification” mode driven via wave-note metadata.

Changes:

  • Introduces SF_PlotterGraphStruct to aggregate plotter state (graph/window, counters, metadata, formatting waves).
  • Refactors metadata/result gathering into SF_FillPlotMetaData() and SF_FillFormulaResults().
  • Adds /Plot (SF_META_PLOT) metadata and a new “full plotting spec” loop path in SF_FormulaPlotter().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

File Description
Packages/MIES/MIES_SweepFormula.ipf Introduces plotter state struct, refactors plotter helpers, and adds full-plotting-spec execution path.
Packages/MIES/MIES_Constants.ipf Adds SF_META_PLOT constant used to flag full plotting specification.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MichaelHuth MichaelHuth force-pushed the feature/2630-use_struct_in_sf_plotter branch from 24783fe to 37d1449 Compare February 9, 2026 20:54
@timjarsky
Copy link
Collaborator

@MichaelHuth, it looks like the avg trace marker color (pink) can be reused. We should reserve pink for the avg trace. Also, it might be worth anticipating the need to change the marker type when plotting multiple average traces on the same plot.

image

@MichaelHuth MichaelHuth changed the title SF plotter use strcuture to aggregate argument data, add support for full plotting spec SF plotter use structure to aggregate argument data, add support for full plotting spec Feb 12, 2026
Copilot AI review requested due to automatic review settings February 12, 2026 18:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MichaelHuth MichaelHuth force-pushed the feature/2630-use_struct_in_sf_plotter branch from 1140201 to 8c00e84 Compare February 13, 2026 11:06
Copilot AI review requested due to automatic review settings February 13, 2026 11:25
@MichaelHuth MichaelHuth force-pushed the feature/2630-use_struct_in_sf_plotter branch from 8c00e84 to b6ecfa9 Compare February 13, 2026 11:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (1)

Packages/MIES/MIES_SweepFormula.ipf:1623

  • SF_AddPlotTraceStyle is declared to return [STRUCT SF_PlotterGraphStruct pg] and uses pg.*, but it neither takes pg as an input nor returns it at the end. This will either fail to compile or return a default/empty struct, and it can’t apply styles to the intended window/state. Update the signature to accept STRUCT SF_PlotterGraphStruct &pg and return consistently (or drop the struct return entirely if you pass by reference).
static Function [STRUCT SF_PlotterGraphStruct pg] SF_AddPlotTraceStyle(variable formulasAreDifferent)

	variable i, j, numTraces, markerCode, lineCode, isCategoryAxis, tagCounter, lineStyle, overrideMarker, traceToFront
	string trace, info, tagText, name, wvName

	for(i = 0; i < pg.formulaCounter; i += 1)
		WAVE/WAVE plotFormData  = pg.collPlotFormData[i]
		WAVE/T    tracesInGraph = plotFormData[0]
		WAVE/WAVE dataInGraph   = plotFormData[1]
		numTraces  = DimSize(tracesInGraph, ROWS)
		markerCode = formulasAreDifferent ? i : 0
		markerCode = SFH_GetPlotMarkerCodeSelection(markerCode)
		lineCode   = formulasAreDifferent ? i : 0
		lineCode   = SFH_GetPlotLineCodeSelection(lineCode)
		for(j = 0; j < numTraces; j += 1)

			WAVE/Z wvX = dataInGraph[j][%WAVEX]
			WAVE   wvY = dataInGraph[j][%WAVEY]
			trace = tracesInGraph[j]

			info           = AxisInfo(pg.win, "left")
			isCategoryAxis = (NumberByKey("ISCAT", info) == 1)

			if(isCategoryAxis)
				WAVE traceColorHolder = wvX
			else
				WAVE traceColorHolder = wvY
			endif

			WAVE/Z traceColor = JWN_GetNumericWaveFromWaveNote(traceColorHolder, SF_META_TRACECOLOR)
			if(WaveExists(traceColor))
				switch(DimSize(traceColor, ROWS))
					case 3:
						ModifyGraph/W=$pg.win rgb($trace)=(traceColor[0], traceColor[1], traceColor[2])
						break
					case 4:
						ModifyGraph/W=$pg.win rgb($trace)=(traceColor[0], traceColor[1], traceColor[2], traceColor[3])
						break
					default:
						FATAL_ERROR("Invalid size of trace color wave")
				endswitch
			endif

			tagText = JWN_GetStringFromWaveNote(wvY, SF_META_TAG_TEXT)
			if(!IsEmpty(tagText))
				name = "tag" + num2str(tagCounter++)
				Tag/C/N=$name/W=$pg.win/F=0/L=0/X=0.00/Y=0.00 $trace, 0, tagText
			endif

			ModifyGraph/W=$pg.win mode($trace)=SF_DeriveTraceDisplayMode(wvX, wvY)

			lineStyle = JWN_GetNumberFromWaveNote(wvY, SF_META_LINESTYLE)
			if(IsValidTraceLineStyle(lineStyle))
				ModifyGraph/W=$pg.win lStyle($trace)=lineStyle
			elseif(formulasAreDifferent)
				ModifyGraph/W=$pg.win lStyle($trace)=lineCode
			endif

			WAVE/Z customMarkerAsFree = JWN_GetNumericWaveFromWaveNote(wvY, SF_META_MOD_MARKER)
			if(WaveExists(customMarkerAsFree))
				DFREF dfrWork = SFH_GetWorkingDF(pg.graph)
				wvName = "customMarker_" + NameOfWave(wvY)
				WAVE customMarker = MoveFreeWaveToPermanent(customMarkerAsFree, dfrWork, wvName)
				ASSERT(DimSize(wvY, ROWS) == DimSize(customMarker, ROWS), "Marker size mismatch")
				ModifyGraph/W=$pg.win zmrkNum($trace)={customMarker}
			else
				overrideMarker = JWN_GetNumberFromWaveNote(wvY, SF_META_MOD_MARKER)

				if(!IsNaN(overrideMarker))
					markerCode = overrideMarker
				endif

				ModifyGraph/W=$pg.win marker($trace)=markerCode
			endif

			traceToFront = JWN_GetNumberFromWaveNote(wvY, SF_META_TRACETOFRONT)
			traceToFront = IsNaN(traceToFront) ? 0 : !!traceToFront
			if(traceToFront)
				ReorderTraces/W=$pg.win _front_, {$trace}
			endif

		endfor
	endfor
End

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MichaelHuth MichaelHuth force-pushed the feature/2630-use_struct_in_sf_plotter branch from b6ecfa9 to 993d7bd Compare February 13, 2026 14:04
Copilot AI review requested due to automatic review settings February 13, 2026 14:09
@MichaelHuth MichaelHuth force-pushed the feature/2630-use_struct_in_sf_plotter branch from 993d7bd to 0676fcc Compare February 13, 2026 14:09
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (2)

Packages/MIES/MIES_SweepFormula.ipf:970

  • SF_CreateTracesForResultsImpl uses pg.graph/pg.plotMetaData/pg.colorGroups/pg.win, but pg is not an input parameter—it's only declared as a return value. This will run with an uninitialized struct and then overwrite the caller’s plotter state on return. Pass the current plotter state in as STRUCT SF_PlotterGraphStruct &pg (and similarly pass counters like dataCnt/gdIndex that are currently implicit).
static Function [variable dataCnt, STRUCT SF_PlotterGraphStruct pg, variable gdIndex, string annotation, variable formulaAddedOncePerDataset] SF_CreateTracesForResultsImpl(WAVE wvResultY, WAVE/Z wvResultX, variable dataNum, variable showInTable, WAVE plotFormData)

	STRUCT RGBColor color
	variable numTraces, yPoints, xPoints, yMxN, xMxN, idx, splitTraces
	variable i, isCategoryAxis, splitX, splitY

Packages/MIES/MIES_SweepFormula.ipf:1153

  • SF_CreateTracesForResults is called to consume pg.formulaResults and update plotter state, but it takes no inputs and relies on pg as a return variable. As a result, pg.formulaResults/pg.wAnnotations/etc. will be uninitialized here. Make pg an explicit input parameter (preferably by reference) and return only real outputs.
static Function [variable dataCnt, STRUCT SF_PlotterGraphStruct pg] SF_CreateTracesForResults()

	variable i, idx, showInTable, numData, formulaAddedOncePerDataset
	variable gdIndex // indexes in tracesInGraph wave and dataInGraph wave in SF_CollectTraceData(), both waves are stored in plotformData
	string annotation = ""

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MichaelHuth MichaelHuth force-pushed the feature/2630-use_struct_in_sf_plotter branch from 0676fcc to b89b463 Compare February 13, 2026 14:31
Copilot AI review requested due to automatic review settings February 13, 2026 14:41
@MichaelHuth MichaelHuth force-pushed the feature/2630-use_struct_in_sf_plotter branch from b89b463 to a867555 Compare February 13, 2026 14:41
@MichaelHuth MichaelHuth removed their assignment Feb 13, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

To own func:
SF_FinishPlotWindow and SF_AddPlotTraceStyle that sets trace styles
- Add constant for SF full plotting meta tag
The tag indicates that the formula result contains a full plotting
specification

- Add SF_IsDataForFullPlotting that checks if a given result has the tag
set. (Tag must be set an nonzero)
A full plotting specification is a wave reference wave tree that
contains formula results in its leafs. The top level iterates over all
formulas specified with AND the second level over all formulas
specified with WITH.
The second level has two columns named FORMULAX and FORMULAY containing the
respective formula results.

The full plot specification is inserted into the top level plots/tables.

If after the operation returning a full plot specification the plot
is continued with WITH, it is respected by the plotter.
An operation "testop" is added that is available when AUTOMATED_TESTING is
defined. The operation calls a function that can be setup in a testcase
for a specific sweepbrowser.
The function can be set like

	SVAR funcName = $GetSFTestopName(graph)
	funcName = "MyTestOp"

this function must have the same signature as a regular SF operation:
    Function/WAVE MyTestOp(STRUCT SF_ExecutionData &exd)

This SF extension allows to implement specific operation behavior in a test
that is not present/exposed to the version of MIES in normal execution.
With the introduction of operation returning a full plotting specification there
are use cases where only variables need to be evaluated without a specific formula.

For that an optional argument allowEmptyCode was added to disable the check for
empty formula code.
…fication

The full plotting specification is a tree of waveref waves where the first level
are AND waves and the second level are WITH waves.
Each element of the AND wave a WITH wave must be assigned.
…ssed

This means:
- The formula string is not preprocessed
- Variable definitions are not allowed
- The variable storage is kept as is and can be used in the formula evaluation
- The error location for SFH_ASSERT is kept, such that for the case of an error
the SF notebook location can be marked

The preProcess option allows to execute a formula "internally" without
changing the variable storage or error information related to the SF notebook.
…nnotation

Changed SF_GetTraceAnnotationText such that a set legendPrefix is always included
in the returned text.
Copilot AI review requested due to automatic review settings February 24, 2026 08:42
@MichaelHuth MichaelHuth force-pushed the feature/2630-use_struct_in_sf_plotter branch from a7b7ccb to 6656bb5 Compare February 24, 2026 08:42
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 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 MichaelHuth force-pushed the feature/2630-use_struct_in_sf_plotter branch from 6656bb5 to a4fac13 Compare February 24, 2026 10:52
@t-b
Copy link
Collaborator

t-b commented Feb 24, 2026

@MichaelHuth LGTM. CI fails though.

This is useful for serializing a text wave
…e same plot

The conversion from textwave to list caused an addition of another CR.
This is solved with a wave conversion that simply serializes the text wave.
The test defines a testop() that creates a specific full plotting specification.
Then the operation is positioned in the outer SF code connected with
AND and WITH before and after to the surrounding code.
Also meta data transfer from the dataset and data wave is checked through
SF_META_YAXISLABEL and SF_META_LEGEND_LINE_PREFIX
This function allows to add a result wave to the variable storage.
If the specified entry already exists it is overwritten.
and using local variable storages
and adding variables in an operation
document testop
Copilot AI review requested due to automatic review settings February 24, 2026 14:27
@MichaelHuth MichaelHuth force-pushed the feature/2630-use_struct_in_sf_plotter branch from a4fac13 to 388d140 Compare February 24, 2026 14:27
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@t-b t-b self-requested a review February 24, 2026 15:17
@t-b t-b enabled auto-merge February 24, 2026 15:17
@t-b t-b merged commit 84e7f45 into main Feb 24, 2026
22 checks passed
@t-b t-b deleted the feature/2630-use_struct_in_sf_plotter branch February 24, 2026 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Split data structure introduction for SF plotter to own PR

4 participants