Skip to content

[MAINT] Use GuiUtil.communicator wherever possible#3863

Open
krzywon wants to merge 4 commits intomainfrom
single-communicator
Open

[MAINT] Use GuiUtil.communicator wherever possible#3863
krzywon wants to merge 4 commits intomainfrom
single-communicator

Conversation

@krzywon
Copy link
Contributor

@krzywon krzywon commented Feb 6, 2026

Description

This ensures all cross widget signals are directed through the same signal handler. This prevents orphaned signals and cleans up the interface for generating signals. I am not 100% sure I found all communicator objects, but found many.

How Has This Been Tested?

This needs more testing, especially in places where self.<parent|manager>.<communicate|communicator> are being replaced.

Review Checklist:

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)

…municator to ensure every widget is calling the same communicator object to ensure all signals are passed through a central object
@llimeht
Copy link
Contributor

llimeht commented Feb 7, 2026

I always found the noun vs verb in communicator vs communicate quite confusing in this code base. That might not be worth addressing at all, of course.

It probably is worth doing something with the tests, although since they got disabled a few years ago the bit rot will be terrible. That said, this is the sort of change that is likely to help them greatly by fixing cross-thread calls that Qt does not support and so led to segfaults in them.

@rozyczko
Copy link
Member

rozyczko commented Feb 7, 2026

Addressing this syntax difference is useful - thanks for doing this, @krzywon!
I agree with @llimeht about noun vs. verb issue - should we standardize on (possibly better fitting) noun here?
Using communicator as the instance of Communicate class would probably be more correct.

@krzywon
Copy link
Contributor Author

krzywon commented Feb 9, 2026

I switched the base import to GuiUtils.communicator (but kept the class as Communicate) and removed all communicate | self.communicate variables from classes, including from unit tests. I think everything is ready for testing at this point, but this might require some extra care to be sure all signals are triggering properly.

@krzywon krzywon changed the title [MAINT] Use GuiUtil.communicate wherever possible [MAINT] Use GuiUtil.communicator wherever possible Feb 10, 2026
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.

3 participants