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

Patch/11bit #102

Merged
merged 4 commits into from
Feb 22, 2023
Merged

Patch/11bit #102

merged 4 commits into from
Feb 22, 2023

Conversation

ieivanov
Copy link
Collaborator

Patch to help me read and write 11bit NDTiff datasets. This is not a permanent solution to micro-manager/pycro-manager#536. The reader and writer should not enumerate all possible pixel types. Instead, for example, pixel types between 8-bit and 16-bit should all be written as uint16 data type.

@henrypinkard
Copy link
Member

Ah good insight that this is the source of the problem!

Instead, for example, pixel types between 8-bit and 16-bit should all be written as uint16 data type.

This is how it works already, the reason the other bit depths are their own pixel types is that they are an "essential" piece of metadata belonging to the image (like, width and height) that should be kept separate from other image metadata, because they are required for things to work correctly, and there is no mechanism for specifying required metadata currently other than adding it to the API.

However, this was initially not how I designed things, so in order to make things work without making backwards incompatible changes to NDTiff I added these other pixels types.

There's definitely a larger discussion to be had about this essential image metadata in concert with the next iteration of file saving (micro-manager/mmCoreAndDevices#323).

But for now I think this (and maybe enumerating the other pixel types as well) are the cleanest way to keep everything properly working.

I will also look into why a more useful error wasn't generated in relation to this.

@henrypinkard henrypinkard merged commit 6917bcc into micro-manager:main Feb 22, 2023
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