Skip to content

3500: Fix for PD plotting#3505

Merged
krzywon merged 2 commits intomainfrom
3500-pd-distribution-plots
Jul 30, 2025
Merged

3500: Fix for PD plotting#3505
krzywon merged 2 commits intomainfrom
3500-pd-distribution-plots

Conversation

@krzywon
Copy link
Copy Markdown
Contributor

@krzywon krzywon commented Jul 15, 2025

Description

This restores some of the code changed during the fitting widget refactor (#3169) to ensure the PD plots are correct when switching models, in any way. This 2nd commit (7c0569c) restores the PD function on model change, which wasn't present before.

Fixes #3500
(Possibly?) Fixes #3248
(Possibly?) Fixes #2540
(Possibly?) Fixes #1587
(Possibly?) Fixes #3506

How Has This Been Tested?

I repeated the steps in #3500, selecting schultz and switching between sphere and core_shell_sphere. The PD distribution is nominally correct, and the PD function remains as schulz. I also added and changed the structure factor and the function remained schultz for each S(Q) change.

I have NOT tested loading a project, but this is the same method called during the project load process, so I am guessing this may also fix the issues related to that.

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)

@dehoni
Copy link
Copy Markdown
Contributor

dehoni commented Jul 16, 2025

Tested and fixes #3506, #1587, #2540, #3248

@dehoni
Copy link
Copy Markdown
Contributor

dehoni commented Jul 17, 2025

#3741 seems to be a related issue?

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 and behaves as expected with the caveat explained below

self.poly_model.item(row, 5+joffset).setText(param_repr)
# Function
param_repr = param_dict[param_name][6+ioffset]
self.poly_model.item(row, 6+joffset).setText(param_repr)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Setting the text on the combobox is not necessary.
Column 6 always contains a combo box (addNameToPolyModel)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I had to add this, otherwise the combo box was never updated when loading projects or changing models.

@rozyczko
Copy link
Copy Markdown
Member

rozyczko commented Jul 24, 2025

This fixes the immediate issue of the combobox state not being restored on model change.

However, there's another issue here that needs addressing at some point.
Running a fit with polydisperse parameters checked adds an error column to the polydisp table.
This is OK when just running the fit - the min/max/npts/nsigs/function combobox columns get shifted and display ok.
But when a model is changed AFTER the fitting, the table is not properly reindexed.

The table headers are correctly not displaying the error, but the parameter values still contain the error column.
This means Min now shows the error, Max shows the min value etc. Function is now showing the value of Nsigs and Filename contains the combobox of functions.

#3536

Copy link
Copy Markdown

@gnsmith gnsmith left a comment

Choose a reason for hiding this comment

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

These changes implement the expected behavior on #3500.

@krzywon krzywon merged commit 17e312e into main Jul 30, 2025
30 checks passed
@krzywon krzywon deleted the 3500-pd-distribution-plots branch July 30, 2025 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

4 participants