glencoesoftware / raw2ometiff

Raw format to OME-TIFF converter
GNU General Public License v2.0
46 stars 20 forks source link

Add reset methods for each option #111

Closed melissalinkert closed 9 months ago

melissalinkert commented 10 months ago

Corresponds to https://github.com/glencoesoftware/bioformats2raw/pull/219

melissalinkert commented 10 months ago

Changes look fine, though between this and the getters/setters could I check whether the tileQueue size limit should be being adjusted when maxWorkers is changed? It currently looks like this is only set when the class is initialised.

Good catch; a0dafc7 should reset tileQueue to match maxWorkers when the setter or resetter actually changes the value of maxWorkers.

DavidStirling commented 9 months ago

I'm a bit lost here now, how are we meant to reset values if not using the CLI?

DavidStirling commented 9 months ago

While cmd.parseArgs() works well as a resetter in the bioformats2raw PR, trying to call it with raw2ometiff is generating the following error:

picocli.CommandLine$MissingParameterException: Missing required parameters: '<inputPath>', '<outputPath>'
    at picocli.CommandLine$Interpreter.assertNoMissingParameters(CommandLine.java:14918)
    at picocli.CommandLine$Interpreter.validateConstraints(CommandLine.java:13653)
    at picocli.CommandLine$Interpreter.parse(CommandLine.java:13614)
    at picocli.CommandLine$Interpreter.parse(CommandLine.java:13559)
    at picocli.CommandLine$Interpreter.parse(CommandLine.java:13454)
    at picocli.CommandLine.parseArgs(CommandLine.java:1552)
    at com.glencoesoftware.convert.tasks.CreateTiff.<init>(CreateTiff.java:64)
    at com.glencoesoftware.convert.workflows.ConvertToTiff.<init>(ConvertToTiff.java:25)
    ... 43 more

I can't see anything significantly different in the CLI implementation, aside from the underlying i/o variables being Path objects instead of Strings. Not sure if that could be causing it to reject.

melissalinkert commented 9 months ago

@DavidStirling: sorry, the build was failing quietly here due to a merge conflict, so it's likely you're seeing MissingParameterException because the relevant default value changes weren't included. f996b88 fixes the conflict and builds now pass, so see if it works after updating?