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

Refactor roi table widget #2509

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

Conversation

samtygier-stfc
Copy link
Collaborator

@samtygier-stfc samtygier-stfc commented Feb 25, 2025

Work on #2378

Description

Refactor ROITableWidget

Changes

  • Remove unneeded code
    • get_row_data()
    • Error checking in remove_roi()
    • Check for rows in __init__
  • Change properties to attributes
    • properties to attributes
  • Move on_row_change into class

Needs rebase after #2506

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

Documentation and Additional Notes

@coveralls
Copy link

coveralls commented Feb 25, 2025

Coverage Status

coverage: 72.897% (+0.05%) from 72.846%
when pulling b632df8 on 2378-refactor-roi-table-widget
into c409e60 on main.

@samtygier-stfc samtygier-stfc force-pushed the 2378-refactor-roi-table-widget branch from 54b2947 to e04a5b1 Compare February 25, 2025 09:50
…e_roi()

self.table_view.get_row_data() would have given an IndexError if the index was wrong, so existing error checking would never have been reached. Safe to assume that this never gets called with an invalid index.
No rows are created in the constructor
@samtygier-stfc samtygier-stfc force-pushed the 2378-refactor-roi-table-widget branch from e04a5b1 to b632df8 Compare February 26, 2025 14:10
@samtygier-stfc samtygier-stfc marked this pull request as ready for review February 26, 2025 14:11
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.

2 participants