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

Spectrum viewer refactoring - Moving logic from the view to the presenter #2227

Merged
merged 10 commits into from
Jun 17, 2024

Conversation

MikeSullivan7
Copy link
Collaborator

Issue

Closes #2224

Description

The following functions have been moved from the view to the presenter:

  • do_adjust_roi
  • handle_storing_current_roi_name_on_tab_change
  • parts of on_visibility_change

some new functions have been made to help with testing:

  • presenter.check_action
  • view.get_roi_properties_spinboxes
  • view.setup_roi_properties_spinboxes

The logic for presenter.handle_storing_current_roi_name_on_tab_change has also been simplified.

Unit tests for the following functions have been added:

  • presenter.change_selected_menu_option
  • presenter.do_adjust_roi
  • presenter.handle_storing_current_roi_name_on_tab_change
  • presenter.refresh_spectrum_plot
  • presenter.redraw_all_rois
  • presenter.handle_roi_clicked
  • handle_enable_normalised

Testing

make check

Acceptance Criteria

Open MI and load some data into it (with spectrum file if possible) and make sure that the functionality is the same as it was before this refactor and no bugs have been introduced.

run make check or make test and check that all tests pass

Documentation

Will add release note

@MikeSullivan7 MikeSullivan7 force-pushed the 2224_spectrum_viewer_refactoring branch from 537961b to a0b1e70 Compare June 13, 2024 15:08
@coveralls
Copy link

Coverage Status

coverage: 72.978% (+0.3%) from 72.705%
when pulling 529ee6c on 2224_spectrum_viewer_refactoring
into 06f179e on main.

@MikeSullivan7 MikeSullivan7 marked this pull request as ready for review June 13, 2024 15:50
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.

Not a full review, just a quick thing that i noticed

mantidimaging/gui/windows/spectrum_viewer/presenter.py Outdated Show resolved Hide resolved
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.

All looks good. Passes tests.

@coveralls
Copy link

Coverage Status

coverage: 72.975% (+0.3%) from 72.705%
when pulling b09bebe on 2224_spectrum_viewer_refactoring
into 06f179e on main.

@samtygier-stfc samtygier-stfc added this pull request to the merge queue Jun 17, 2024
Merged via the queue into main with commit fd4e527 Jun 17, 2024
8 checks passed
@samtygier-stfc samtygier-stfc deleted the 2224_spectrum_viewer_refactoring branch June 17, 2024 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Move logic from Spectrum Viewer view to presenter to make it testable
3 participants