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

FLASH PR Refactor How Fits Files are Loaded to Remove String Formatting #2462

Merged
merged 1 commit into from
Jan 20, 2025

Conversation

JackEAllen
Copy link
Collaborator

@JackEAllen JackEAllen commented Jan 17, 2025

Issue

FLASH PR

Description

Change implementation of how .fits files are read into the live viewer (mantidimaging/gui/windows/live_viewer/presenter.py:112).

Using __str__(), which is a special method directly which is normally used by str() does not respect the encapsulation of the object's implementation and can be less explicit. This in turn means using __str__() could be considered less Pythonic. This PR removes the use of __str__() as string formatting is not necessary.

As load_image() states that an np.ndarray should be returned, I have also assigned image_data to an empty array (np.array([])) to cover the case where neither a .tif, .tiff or .fits file is in the suffix of the path used as an argument to load_image() to resolve 'image_data' before assignment - Pylint where image_data was not previously assigned if neither case was met.

Testing

Describe the tests that you were used to verify your changes.

  • Checked that unit tests pass
  • Tested live viewer with simulate live viewer utility script (scripts/simulate_live_data.py) with .fits files.

Acceptance Criteria

Acceptance criteria for reviewer to tick off in order to verify your changes.

  • .fits files can correctly be loaded and read by live viewer
    • Verify that opening a pre-existing dataset in the live viewer that uses .fits still works as it did on main
    • Verify using the scripts/simulate_live_data.py script that the live viewer loads in .fits files as they arrive in a selected folder

Documentation

How have you changed the documentation to reflect your changes? All changes should be noted in the appropriate file in docs/release_notes

N/A not needed.

@JackEAllen JackEAllen self-assigned this Jan 17, 2025
@coveralls
Copy link

coveralls commented Jan 17, 2025

Coverage Status

coverage: 73.6% (-0.009%) from 73.609%
when pulling c2f20e5 on live_view_read_fits
into a2e5302 on main.

@JackEAllen JackEAllen changed the title FLASH PR Refactor How Fits File is Read FLASH PR Refactor How Fits Files are Loaded to Use POSIX Jan 17, 2025
@JackEAllen JackEAllen marked this pull request as ready for review January 20, 2025 14:39
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.

Looks good, and works locally.

@JackEAllen JackEAllen changed the title FLASH PR Refactor How Fits Files are Loaded to Use POSIX FLASH PR Refactor How Fits Files are Loaded to Remove String Formatting Jan 20, 2025
@samtygier-stfc samtygier-stfc added this pull request to the merge queue Jan 20, 2025
Merged via the queue into main with commit 92c4cfc Jan 20, 2025
8 checks passed
@samtygier-stfc samtygier-stfc deleted the live_view_read_fits branch January 20, 2025 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants