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

Fix the type for acquisitions.id under the plate metadata #152

Merged
merged 3 commits into from
May 19, 2022

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented May 5, 2022

Although the type is not specified in https://ngff.openmicroscopy.org/0.4/#plate-md, the snippets use integers for the aqcuisitions identifiers. The well.acquisition metadata is also of integer type to create a reference to the acquisitions objects defined at the plate level.

39237e2 updates the current writer code and adjusts the tests accordingly. Note this PR is currently built on top of #149 which adds a new HCS test with the previous behavior for the id type.

@sbesson
Copy link
Member Author

sbesson commented May 6, 2022

@melissalinkert I also had a quick look at raw2ometiff to see whether this would require an associated PR but I think plate.acquisitions.id is only consumed in https://github.com/glencoesoftware/raw2ometiff/blob/master/src/main/java/com/glencoesoftware/pyramid/PyramidFromDirectoryWriter.java#L376 and the usage of toString() should work with Integer.
In order to run a minimal test, are the current HEAD of bioformats2raw and raw2ometiff compatible at the moment or is anything required on the raw2ometiff front to consume the 0.5.0-SNAPSHOT changes?

Copy link
Member

@melissalinkert melissalinkert left a comment

Choose a reason for hiding this comment

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

I'm fine with this, but it probably makes sense to update the spec as well to clarify the correct type.

@melissalinkert
Copy link
Member

I'd expect data generated by current HEAD of bioformats2raw to work with current HEAD of raw2ometiff. I wouldn't try to update the bioformats2raw dependency in raw2ometiff locally (due to jzarr version differences).

Keep in mind though that https://github.com/glencoesoftware/raw2ometiff/blob/master/src/main/java/com/glencoesoftware/pyramid/PyramidFromDirectoryWriter.java#L376 will only be executed if there is no OME-XML metadata file:

https://github.com/glencoesoftware/raw2ometiff/blob/master/src/main/java/com/glencoesoftware/pyramid/PyramidFromDirectoryWriter.java#L624

@sbesson
Copy link
Member Author

sbesson commented May 13, 2022

See ome/ngff#120 for the OME-NGFF JSON schema PR specifying the type of acquisition.id

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