Skip to content

Fix for C++ conversion error in plotting#3745

Merged
jellybean2004 merged 3 commits into
mainfrom
tab_id_patch
Nov 13, 2025
Merged

Fix for C++ conversion error in plotting#3745
jellybean2004 merged 3 commits into
mainfrom
tab_id_patch

Conversation

@jellybean2004
Copy link
Copy Markdown
Member

Description

Fixes #3740

The issue occurred because None was passed as a parameter to plotRequestedSignal.
In src/sas/qtgui/Utilities/GuiUtils.py:106, the signal is defined to accept list and int. Since None is not an int, it triggered a C++ type conversion error when the signal was emitted, as Qt is implemented in C++.

PySide does not support Python-style union type hints (e.g. int | None) in signal definitions, so supporting both types directly is a rather cumbersome implementation.
As a workaround, the fix passes a valid integer value instead of None:

  • 0 for perspectives that do not support tabs,

  • self.tab_id for those that do.

Although a deeper look showed that tab_id is not actually used in the connected slot, it has been left intact to preserve the existing signal interface.

How Has This Been Tested?

Checked by plotting in all the affected perspectives (SizeDistribution, Inversion, Invariant). The error does not appear anymore.

Review Checklist:

[if using the editor, use [x] in place of [ ] to check a box]

Documentation (check at least one)

  • There is nothing that needs documenting
  • Documentation changes are in this PR
  • There is an issue open for the documentation (link?)

Installers

  • There is a chance this will affect the installers, if so
    • Windows installer (GH artifact) has been tested (installed and worked)
    • MacOSX installer (GH artifact) has been tested (installed and worked)
    • Wheels installer (GH artifact) has been tested (installed and worked)

Licensing (untick if necessary)

  • The introduced changes comply with SasView license (BSD 3-Clause)

Copy link
Copy Markdown
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

I'm slightly confused how this could be the solution.
plotRequestedSignal's only slot is in GuiManager

self.communicate.plotRequestedSignal.connect(self.showPlot)

showPlot passes the second argument (id) to displayData, not doing anything to it.

self.filesWidget.displayData(plot, id)

Sunsequently, displayData, although accepting the argument

def displayData(self, data_list, id=None):

does absolutely nothing with it. The whole id passing chain is completely redundant at this point (it used to be a few rewrites/fixes earlier though!)

@rozyczko rozyczko self-requested a review November 12, 2025 14:47
Copy link
Copy Markdown
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

Never mind. I didn't read your description properly :) This makes sense in the context of proper type checks.
However, you might want to consider just removing this argument throughout, so no conversion is taking place. Nor confusion

@jellybean2004
Copy link
Copy Markdown
Member Author

Since the tab_id parameter is obsolete, I have removed it from the definition and all the calls.

Copy link
Copy Markdown
Contributor

@DrPaulSharp DrPaulSharp left a comment

Choose a reason for hiding this comment

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

All looks good, rebase and merge when ready.

Copy link
Copy Markdown
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

Looks good now!

Copy link
Copy Markdown
Contributor

@wpotrzebowski wpotrzebowski left a comment

Choose a reason for hiding this comment

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

Tested on Mac from installer. It works as expected

@jellybean2004 jellybean2004 merged commit c2c96a4 into main Nov 13, 2025
30 checks passed
@jellybean2004 jellybean2004 deleted the tab_id_patch branch November 13, 2025 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

C++ error upon Invariant extrapolation

4 participants