-
Notifications
You must be signed in to change notification settings - Fork 7
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
Refactoring for SpectrumROI #2506
Conversation
dcab92b
to
f4df71e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should self.roi_dict: dict[str, ROI] = {}
in SpectrumWidget
not be self.roi_dict: dict[str, SpectrumROI] = {}
as we use SpectrumROI
methods on it, e.g. adjust_spec_roi()
.
Also we have roi_dict: dict[str, ROI]
on line 112 and self.roi_dict: dict[str, ROI] = {}
on line 141 which is unnecessary.
Good spot. That gives some stricter type checking else where, so needed to fix the type for colour in a few places. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All changes look good, code is a lot more streamlined and easier to read now. All tests pass and Spectrum Viewer behaves as expected when going through usual workflows of using the ROIs.
Work on #2378
Description
Refactoring for
SpectrumROI
Remove some unnecessary methods simplify others
Needs rebase after #2500Developer Testing
I have tested manually and run system tests
Acceptance Criteria and Reviewer Testing
python -m pytest -vs
Documentation and Additional Notes
Not needed