search

ome / ome2024-ngff-challenge

Project planning and material repository for the 2024 challenge to generate 1 PB of OME-Zarr data
https://pypi.org/project/ome2024-ngff-challenge/
BSD 3-Clause "New" or "Revised" License
11 stars 8 forks source link

resave main() returns nothing to indicate no error #43

Closed will-moore closed 2 weeks ago

will-moore commented 2 weeks ago

Trying to run resave using nextflow. Although the conversion appears successful, nextflow thinks there's an error because a non-zero value is returned from the main() function that is called by resave command.

This updates main to return nothing.

Nextflow output ``` $ ../nextflow run --input idr0054_images.tsv --column 1 --idrId idr0054 --modality "NCIT_C182027" --organism NCBI:txid9606 --name "idr0054" --shards "1,1,1,3072,3072" --chunks "1,1,1,1024,1024" resave.nf N E X T F L O W ~ version 24.04.4 Launching `resave.nf` [irreverent_sammet] DSL2 - revision: 6bfda73597 executor > local (3) [41/550457] process > convert (2) [ 0%] 0 of 3 executor > local (3) [41/550457] process > convert (2) [100%] 1 of 1, failed: 1 [- ] process > upload - [- ] process > remove - ERROR ~ Error executing process > 'convert (3)' Caused by: Process `convert (3)` terminated with an error exit status (1) Command executed: ome2024-ngff-challenge resave --input-bucket=bia-integrator-data --input-endpoint=https://uk1s3.embassy.ebi.ac.uk --input-anon "S-BIAD800/f49ada41-43bf-47ff-99b9-bdf8cc311ce3/f49ada41-43bf-47ff-99b9-bdf8cc311ce3.zarr" "Tonsil 3.zarr" --log error --rocrate-modality=NCIT_C182027 --rocrate-organism=NCBI:txid9606 --rocrate-name=idr0054 --cc-by --output-shards=1,1,1,3072,3072 --output-chunks=1,1,1,1024,1024 Command exit status: 1 Command output: (empty) Command error: i: 0%| | 0/1 [00:00
will-moore commented 2 weeks ago

I see that tests are expecting the return value to confirm that correct numbers of images have been converted. Do we want to preserve the option of returning that number for testing purposes (or any other reasons)? If so, I could add that as an option?

I would like to check first whether my change actually fixes the nextflow...

joshmoore commented 2 weeks ago

Do we want to preserve the option of returning that number for testing purposes?

Yes, please.

If so, I could add that as an option?

Don't really care too much which strategy is used. A second method that doesn't return is also fine.

joshmoore commented 2 weeks ago

You still do want the SystemExit behavior for nextflow?

will-moore commented 2 weeks ago

@joshmoore I don't know yet that I want any change there... I guess there might be a case for failing silently if nothing is converted, so that nextflow doesn't fall-over mid-way through a bunch of jobs. But I don't know if changing the SystemExit behaviour would make the difference there to allow that.

joshmoore commented 2 weeks ago

Fair enough. I've added a mention of nextflow since the --silent flag actually has no impact on the regular user. I'll get this out as 1.0.2 ASAP.

joshmoore commented 2 weeks ago

NB: might be worth updating the README with a nextflow example once things are stable.