Skip to content

Conversation

@rozyczko
Copy link
Member

Description

Before: The code used a try/except approach to blindly attempt disconnecting signals, catching RuntimeError exceptions when signals weren't connected. We tried three ways to check signal connection status, which didn't work with Qt 6.2.4.

After: The code now uses the proper isSignalConnected() method with QMetaMethod.fromSignal(), which is the official and clean way to check if a signal is connected before attempting to disconnect it.

The proper usage pattern is

signal_method = QtCore.QMetaMethod.fromSignal(obj.someSignal)
if obj.isSignalConnected(signal_method):
    obj.someSignal.disconnect()

as per https://doc.qt.io/qtforpython-6/PySide6/QtCore/QObject.html#PySide6.QtCore.QObject.isSignalConnected

Fixes #3205

How Has This Been Tested?

Local testing

Review Checklist:

Documentation (check at least one)

  • [X ] 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)

@wpotrzebowski
Copy link
Contributor

It is bit difficult to test it. It seems to work e.g. no errors when closing windows

I am getting
07:53:33 - ERROR: Traceback (most recent call last): File "sas/qtgui/Perspectives/Fitting/ConstraintWidget.py", line 1133, in showMultiConstraint File "sas/qtgui/Perspectives/Fitting/ComplexConstraint.py", line 40, in __init__ File "sas/qtgui/Perspectives/Fitting/ComplexConstraint.py", line 79, in setupWidgets File "sas/qtgui/Perspectives/Fitting/ComplexConstraint.py", line 105, in setupParamWidgets IndexError: list index out of range
when clicking on Add Constraints button but it seems to be unrelated to that

@rozyczko
Copy link
Member Author

Thanks @wpotrzebowski . Can you please tell me how you got this exception from Add Constraints?
I'd like to make sure this change didn't introduce it.

@wpotrzebowski
Copy link
Contributor

Thanks @wpotrzebowski . Can you please tell me how you got this exception from Add Constraints? I'd like to make sure this change didn't introduce it.

It simply happens when trying to "Add Constraints"

@rozyczko
Copy link
Member Author

Thanks @wpotrzebowski . Can you please tell me how you got this exception from Add Constraints? I'd like to make sure this change didn't introduce it.

It simply happens when trying to "Add Constraints"

I have downloaded the installer and tested the functionality - the constraints work well. Can you please test the installer?

@wpotrzebowski
Copy link
Contributor

Ok, I see what I did... I didn't choose any model and that's when it fails. When models are selected, it works as expected. I guess it is a new bug?

@rozyczko
Copy link
Member Author

Ok, I see what I did... I didn't choose any model and that's when it fails. When models are selected, it works as expected. I guess it is a new bug?

Please show how you got this error - I can't reproduce the issue in this or in other branches.

@wpotrzebowski
Copy link
Contributor

Hope, these screenshots help:

Screenshot 2025-11-19 at 14 33 57 Screenshot 2025-11-19 at 14 33 44

@krzywon
Copy link
Contributor

krzywon commented Nov 19, 2025

I'm seeing the same error as @wpotrzebowski, but I tested the procedure outlined below in v6.1.1 and see the same error there as well. I think this is an existing, but previously uncaught, issue, so it shouldn't hold this PR, IMO.

Steps to reproduce:

  1. Open the Const/Simul Fit Window
  2. Load at least two data sets and send both to fitting.
  3. The Add constraints is now active in the Const/Simul Window. Press it!
  4. Error

It seems like the constrained fit panel is enabling the constraint button before a model is selected in the fit pages.

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.

Remove obsolete logic from ConstraintWidget

4 participants