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

BioTek: refine field and channel name regexes #158

Merged
merged 2 commits into from
Sep 23, 2022

Conversation

melissalinkert
Copy link
Member

This allows a negative field index and more punctuation characters in the channel name.

This allows a negative field index and more punctuation characters in the channel name.
@melissalinkert melissalinkert requested a review from muhanadz July 28, 2022 16:31
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.

Testing with some issue example files.
Without PR:

Exception in thread "main" picocli.CommandLine$ExecutionException: Error while calling command (com.glencoesoftware.bioformats2raw.Converter@53fe15ff): java.lang.IllegalArgumentException: Invalid series: 0
	at picocli.CommandLine.executeUserObject(CommandLine.java:1962)
	at picocli.CommandLine.access$1300(CommandLine.java:145)
	at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2352)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2346)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2311)
	at picocli.CommandLine$AbstractParseResultHandler.handleParseResult(CommandLine.java:2172)
	at picocli.CommandLine.parseWithHandlers(CommandLine.java:2550)
	at picocli.CommandLine.parseWithHandler(CommandLine.java:2485)
	at picocli.CommandLine.call(CommandLine.java:2761)
	at com.glencoesoftware.bioformats2raw.Converter.main(Converter.java:2013)
Caused by: java.lang.IllegalArgumentException: Invalid series: 0
	at loci.formats.FormatReader.seriesToCoreIndex(FormatReader.java:1298)
	at loci.formats.FormatReader.setSeries(FormatReader.java:969)
	at loci.formats.MetadataTools.populatePixels(MetadataTools.java:203)
	at loci.formats.MetadataTools.populatePixels(MetadataTools.java:116)
	at com.glencoesoftware.bioformats2raw.BioTekReader.initFile(BioTekReader.java:398)
	at loci.formats.FormatReader.setId(FormatReader.java:1443)
	at loci.formats.ImageReader.setId(ImageReader.java:849)
	at com.glencoesoftware.bioformats2raw.Converter.getBaseReaderClass(Converter.java:1961)
	at com.glencoesoftware.bioformats2raw.Converter.convert(Converter.java:517)
	at com.glencoesoftware.bioformats2raw.Converter.call(Converter.java:493)
	at com.glencoesoftware.bioformats2raw.Converter.call(Converter.java:97)
	at picocli.CommandLine.executeUserObject(CommandLine.java:1953)
	... 9 more

With PR:
Problem example files convert successfully which then could be converted to ome.tiff using raw2ometiff and imported to OMERO successfully.

@chris-allan
Copy link
Member

Before we merge this I'd like to see the regexes unit tested so we know exactly what we're matching and don't end up with regressions down the line when we inevitably need to update them.

@melissalinkert
Copy link
Member Author

f3fe218 adds tests that should fail if we break regex handling in the future. We can obviously add more tests cases there as needed.

@muhanadz, for reviewing the latest commit:

gradlew clean build will automatically run the new tests, since the new test class is under src/test/java. See https://docs.gradle.org/current/userguide/java_testing.html#using_junit5 for more details on how Gradle finds tests.

Once the build finishes, a report of which tests were run and whether they passed or failed will be in build/reports/tests/test. The easiest way to read those is to open build/reports/tests/test/index.html in a web browser and then click through to the specific class name you want to check. In this case, make sure that the BioTekTest class shows up in the report. Each test that appears in the source code should be listed in the class report, and should be listed as passing. You can definitely experiment locally with modifying the tests or the reader slightly to see what happens when things fail (e.g. change an assertFalse to assertTrue, check that a plate count is negative, modify one of the reader's regexes).

getTestCases() lists all of the file names that will be checked; it would be a good idea to check our existing BioTek data and verify that I didn't miss any known naming conventions.

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.

Build is successful with all the tests passing.

To the best of my knowledge, and looking at our biotek data, these tests are sufficient, however, would be good to get someone else's opinion on it.

@chris-allan chris-allan merged commit 35067d6 into glencoesoftware:master Sep 23, 2022
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