Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NaN warnings activated in the Spectrum Viewer #2186

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MikeSullivan7
Copy link
Collaborator

Issue

Closes #2185

Description

In the SpectrumWidget class, the MainWindow is now passed to the SpectrumProjectionWidget and the BadDataOverlay mixin class is used in the SpectrumProjectionWidget. The warning icon now appears in the Spectrum viewer when a slice which has a NaN in it is shown on the averaged image (as the NaN will propagate). The warning icon retains the same functionality as in the Main Window where it can be clicked on to open the Operations Window such that NaN removal can be performed.

Testing

make check

Acceptance Criteria

  • Open MI with data which has NaNs in it (e.g. the Fe dataset)
  • Open the Spectrum Viewer
  • Check that if a slice in the averaged image has NaNs, then the warning icon appears
  • Test this by changing the Range Control on the Spectrum plot such that a "complete" part of the plot is in the range, and the warning icon should not show.
  • Move the Range Control such that an "empty" part of the plot is in the range, and the warning icon should appear. See screenshots for examples.
  • Check that clicking the warning icon has the correct functionality.

image

image

Documentation

Will add release note

@coveralls
Copy link

coveralls commented May 2, 2024

Coverage Status

coverage: 72.735% (+0.005%) from 72.73%
when pulling b01bc1b on 2185_Spectrum_NaN_detect
into 2a6191e on main.

@@ -113,12 +116,12 @@ class SpectrumWidget(QWidget):
spectrum_plot_widget: SpectrumPlotWidget
image_widget: SpectrumProjectionWidget

def __init__(self) -> None:
def __init__(self, parent: MainWindowView) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we avoid calling this parent here. It has a specific meaning in Qt, and caused issues before. I think best to call it main_window.

Copy link
Collaborator

Choose a reason for hiding this comment

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

See #1989

The key bit is not passing it to the Qt widget constructor, but still best to call it main_window

@samtygier-stfc samtygier-stfc self-requested a review May 16, 2024 08:24
Copy link
Collaborator

@samtygier-stfc samtygier-stfc left a comment

Choose a reason for hiding this comment

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

See comment

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.

Add NaN detection and warning to Spectrum Viewer
4 participants