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.

Sign up for GitHub

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

Jump to bottom

Refactor SpectrumViewerWindow #2514

Merged
merged 8 commits into from
Feb 28, 2025
Merged

Refactor SpectrumViewerWindow #2514

merged 8 commits into from
Feb 28, 2025

Conversation

samtygier-stfc
Copy link
Collaborator

@samtygier-stfc samtygier-stfc commented Feb 27, 2025 •
edited by ashleymeigh2
Loading

Work on #2378

Description

This is mostly removal of attributes and methods that are no longer used.

Note: Needs rebase after #2513

Developer Testing

  • I have verified unit tests pass locally: python -m pytest -vs

Acceptance Criteria and Reviewer Testing

  • Unit tests pass locally: python -m pytest -vs
  • In spectrum viewer
    • Adding and removing ROIs
    • Renaming ROIs
    • Changing ROI colour
    • Resizing ROI from image view or properties
    • Anything that you think might have broken from the removed code

Documentation and Additional Notes

Not needed

@coveralls
Copy link

coveralls commented Feb 27, 2025 •
edited
Loading

Coverage Status

coverage: 73.036% (+0.01%) from 73.023%
when pulling c3b5538 on 2378-refactor-spectrumviewerview
into faaf361 on main.

@samtygier-stfc samtygier-stfc force-pushed the 2378-refactor-spectrumviewerview branch from 37b868e to 98fa68b Compare February 28, 2025 09:30
@samtygier-stfc samtygier-stfc force-pushed the 2378-refactor-spectrumviewerview branch from 98fa68b to c3b5538 Compare February 28, 2025 09:32
@samtygier-stfc samtygier-stfc marked this pull request as ready for review February 28, 2025 09:36
ashleymeigh2
ashleymeigh2 approved these changes Feb 28, 2025
View reviewed changes
Copy link
Collaborator

@ashleymeigh2 ashleymeigh2 left a comment

Choose a reason for hiding this comment

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

The changes look good. Manual checks confirm ROIs function correctly (add/remove, rename, color change, resizing). No issues from removed code.

@ashleymeigh2 ashleymeigh2 added this pull request to the merge queue Feb 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 28, 2025
@samtygier-stfc samtygier-stfc added this pull request to the merge queue Feb 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 28, 2025
@samtygier-stfc
Copy link
Collaborator Author

Not sure why this is failing in the merge queue. I'll try it again

@samtygier-stfc samtygier-stfc added this pull request to the merge queue Feb 28, 2025
Merged via the queue into main with commit 38aff26 Feb 28, 2025
8 checks passed
@samtygier-stfc samtygier-stfc deleted the 2378-refactor-spectrumviewerview branch February 28, 2025 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashleymeigh2 ashleymeigh2 ashleymeigh2 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@samtygier-stfc @coveralls @ashleymeigh2