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

Fix ROI movement not updating spectrum unless selected #2489

Merged
merged 5 commits into from
Feb 17, 2025

Conversation

ashleymeigh2
Copy link
Collaborator

Issue Closes #2486

Description

  • Fixed an issue where moving an ROI did not update the spectrum plot unless the ROI was selected first.
  • roi_changed signal now correctly emits the SpectrumROI object instead of an ROI or string.
  • Updated handle_roi_moved to process only the moved ROI for efficiency.
  • Ensured remove_roi correctly retrieves and passes the SpectrumROI object to the presenter.

Developer Testing

  • I have verified unit tests pass locally: python -m pytest -vs
  • Manually tested by:
    • Adding an ROI and moving it.
    • Checking that the spectrum updates immediately without needing selection.
    • Removing an ROI and verifying spectrum updates correctly.

Acceptance Criteria and Reviewer Testing

  • Unit tests pass locally: python -m pytest -vs
  • Adding a new ROI updates the spectrum on resize.
  • Moving an existing ROI updates the spectrum in real-time.
  • Removing an ROI updates the spectrum correctly.

Documentation and Additional Notes

@ashleymeigh2 ashleymeigh2 self-assigned this Feb 11, 2025
@samtygier-stfc
Copy link
Collaborator

I'll rebase this to deal with any conflicts with other PRs. Also I'll take the sigRegionChanged connection back out, because that was done for #2155

@coveralls
Copy link

coveralls commented Feb 14, 2025

Coverage Status

coverage: 72.891% (+0.02%) from 72.872%
when pulling ed0a65f on 2486_spec_plot_update
into 34d5abd on main.

Otherwise it does not get removed from the image in presenter.do_remove_roi()
@samtygier-stfc
Copy link
Collaborator

I also made some tweaks to the remove ROI code, as pop() taking the ROI out of spectrum_widget.roi_dict prevent it being removed from the image view. And removed some bits that were tolerating missing things in dictionaries.

If we hit these cases we probably want to know
@samtygier-stfc samtygier-stfc added this pull request to the merge queue Feb 17, 2025
Merged via the queue into main with commit 3e1614f Feb 17, 2025
8 checks passed
@samtygier-stfc samtygier-stfc deleted the 2486_spec_plot_update branch February 17, 2025 10:13
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.

Spectrum plot not updating
4 participants