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

MRC: read IMOD flags to determine int8 vs uint8 pixel type #3975

Merged
merged 2 commits into from
Apr 12, 2023

Conversation

melissalinkert
Copy link
Member

See glencoesoftware/bioformats2raw#198 (comment)

The test file has been uploaded to curated/mrc/bioformats2raw-198. Without this PR, showinf test_uint8.mrc should read the file, but will incorrectly identify it as int8. With this PR, the same test should show a pixel type of uint8.

I would not expect test failures, and this should be safe for a patch release.

cc @blowekamp

@melissalinkert melissalinkert added this to the 6.13.1 milestone Apr 4, 2023
Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

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

Changes looks good and inline with the specification linked from the reader. Nightly CI tests are passing.

I defer on the merging as we are prioritizing the dependencies upgrade work but from my side, this feels like a valuable fix to include in Bio-Formats 6.13.0.

@dgault
Copy link
Member

dgault commented Apr 12, 2023

Changes here look good and inline with the spec. Testing with the new sample file and with the PR included the file is able to read without issue and PixelType = uint8 which is the correct expected value. No other sample files have been impacted by the PR and all builds and repo tests continue to pass. Merging this for inclusion with 6.13.0.

@dgault dgault merged commit 3cbadb8 into ome:develop Apr 12, 2023
@melissalinkert melissalinkert deleted the mrc-uint8 branch September 6, 2024 19:01
This pull request was closed.
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.

3 participants