-
Notifications
You must be signed in to change notification settings - Fork 1
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
CLI for concatenating zarr stores #145
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will vote for simplifying this PR to concatenation without cropping.
I understand that there are many shared parts of concatenation and cropping and that you're trying cut down on I/O here, but I value modularity and clearly named CLI functions over I/O concerns, which can be solved later during pipelining.
I will also suggest concatenate
over concatenate_datasets
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I needed this utility today and it worked well! Thanks @edyoshikun!
- 'hiding' cropping by defaulting parameters to 'all'.
What is the testing requirement on shrimPy? |
I don't think we had a strict requirement, however, I will add a test function that runs the CLI. Thanks @ziw-liu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've used and tested this, and I'm happy with where it's at. I'm fine with merging and moving w/o tests for now (but please go ahead if you're doing tests anyway @edyoshikun). Thanks!
- added test
I added the test and made some changes to make sure the config and cli files have consistent naming w.r.t. to the other CLIs. |
Thanks @edyoshikun! LGTM |
NOTE: this function might have to be ported to iohub or iphub later
This PR adds a CLI for concatenating zarr stores. The main purpose is to merge multiples stores similar to append_channels.
Additionally, since this relies on the
copy_n_paste_czyx()
function to crop the arrays, one can pass cropping parameters for T,Z,Y, and X in the config file.This helps create toy datasets and merge the datasets at the end of our pipelines and assumes that all the datasets have the same folder structure.
Edit:
all
) by default. The config file by still expose the variables for users who want to do the cropping.