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 plate detection to ignore empty plates #118

Merged
merged 3 commits into from
Dec 22, 2021

Conversation

melissalinkert
Copy link
Member

HCS metadata will now only be written by default if there is at least one Plate, which contains at least one Well, which has at least one WellSample that links to an existing Image ID. This means that empty plates will be ignored for the purposes of writing Zarr, but will be preserved in METADATA.ome.xml.

For datasets that have only empty/invalid plates, the default bioformats2raw behavior with this PR should match bioformats2raw --no-hcs without this PR. No test (yet?) as creating an empty plate via fake files isn't possible.

HCS metadata will now only be written by default if there is at least one
Plate, which contains at least one Well, which has at least one WellSample
that links to an existing Image ID.  This means that empty plates will
be ignored for the purposes of writing Zarr, but will be preserved
in METADATA.ome.xml.
@melissalinkert
Copy link
Member Author

After discussion earlier today, a small OME-XML file that gets committed might be the easiest way to test this.

@muhanadz would you be willing to put together a test file? Two examples for reference:

https://downloads.openmicroscopy.org/images/OME-XML/2016-06/single-image.ome.xml
https://downloads.openmicroscopy.org/images/OME-XML/2016-06/hcs.ome.xml

It basically needs to have one Image defined as in the first example, plus an empty Plate element. Running bioformats2raw test.ome.xml test.zarr on the finished test file without this PR should result in weird behavior where the HCS layout (https://ngff.openmicroscopy.org/latest/#hcs-layout) is used but one or more directories are named -1. The same test with this PR should result in the normal non-HCS layout.

Once a file is ready to go, we can add a corresponding test here.

@muhanadz
Copy link
Member

muhanadz commented Nov 9, 2021

Without PR the following section of the XML gets processed and outputs a Zarr file with an HCS layout where one of the directories is named -1.

./bioformats2raw-0.3.0/bin/bioformats2raw test_bioformats2raw_pr118.ome.xml test_withoutPR.zarr --compression=zlib
  <Plate
     ID="Plate:1"
     Name="Control Plate"
     ColumnNamingConvention="letter"
     RowNamingConvention="number"
     Columns="12"
     Rows="8"
     >
  </Plate>

With PR the following warning message is displayed and the resulting Zarr is a non-HCS layout (like --no-hcs) with normal namings:

./bioformats2raw-0.3.1-SNAPSHOT/bin/bioformats2raw test_bioformats2raw_pr118.ome.xml test_withPR.zarr --compression=zlib 
2021-11-08 11:17:52,815 [main] WARN  c.g.bioformats2raw.Converter - Encountered invalid plate #0

Adding more than one empty plate doesn't change the documented behaviour, but only displays Encountered invalid plate # for the number of empty plates. Removing the defined image in the XML results in Invalid series: 0 which is expected with PR and without PR

test_withoutPR_no-hcs.zarr.zip
test_withoutPR.zarr.zip
test_withPR.zarr.zip
test_bioformats2raw_pr118.ome.xml.zip

Copy link
Member

@muhanadz muhanadz left a comment

Choose a reason for hiding this comment

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

From previous review, fixed test.ome.xml file to pass bftools' xmlvalid as it wasn't before

bftools_v6.5.1-397/xmlvalid ./test_bioformats2raw_pr118.ome.xml 
Parsing schema path
http://www.openmicroscopy.org/Schemas/OME/2016-06/ome.xsd
Validating ./test_bioformats2raw_pr118.ome.xml
No validation errors found.

The behaviour with and without the PR remains the same as previously documented with this test.ome.xml file.

test_bioformats2raw_pr118.ome.xml.zip
test_withoutPR.zarr.zip
test_withPR.zarr.zip

@melissalinkert
Copy link
Member Author

That's perfect, thanks @muhanadz. 52d1de0 commits the file and adds the corresponding unit test. /cc @chris-allan

I of course completely forgot that we had already added some small OME-XML tests in #91, so adding another one was easier than expected.

Copy link
Member

@muhanadz muhanadz left a comment

Choose a reason for hiding this comment

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

gradle build on this PR builds fine with no problems.

BUILD SUCCESSFUL in 1m 55s
11 actionable tasks: 11 executed

@chris-allan chris-allan merged commit b2c30c5 into glencoesoftware:master Dec 22, 2021
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