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

Append channels via CLI calls #454

Merged
merged 13 commits into from
Nov 1, 2023
Merged

Append channels via CLI calls #454

merged 13 commits into from
Nov 1, 2023

Conversation

talonchandler
Copy link
Collaborator

@talonchandler talonchandler commented Oct 31, 2023

Before this PR:
This pair of CLI calls

recorder reconstruct -i input.zarr/0/0/0 -c ./phase.yaml -o output.zarr
recorder reconstruct -i input.zarr/0/0/0 -c ./DAPI.yaml -o output.zarr

would overwrite the first phase reconstruction, resulting in a DAPI-only reconstruction.

After this PR:
The same CLI calls result in two channels in output.zarr as expected.

Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Merging #454 (c3379a1) into main (2aa080e) will increase coverage by 0.61%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##            main    #454      +/-   ##
========================================
+ Coverage   8.83%   9.45%   +0.61%     
========================================
  Files         29      29              
  Lines       4561    4592      +31     
========================================
+ Hits         403     434      +31     
  Misses      4158    4158              
Files Coverage Δ
recOrder/tests/cli_tests/test_reconstruct.py 97.87% <100.00%> (+0.72%) ⬆️
recOrder/tests/util_tests/test_create_empty.py 100.00% <100.00%> (ø)

@talonchandler talonchandler marked this pull request as ready for review October 31, 2023 23:36
@edyoshikun
Copy link
Contributor

This appending channels is also something that I was battling with mantis, especially after the registration as well. I have found that appending is tricky when the process of appending crashes ( for whatever reason, you accidentally close the terminal, end the process,etc) then if you try to restart the appending since the channel name already exists it will fail. I was running both separately to make sure I had successful reconstructions/deskewing and then appending together. Is there a an easier way to cleanup or to re-append?

@Soorya19Pradeep
Copy link

Thank you @talonchandler for implementing cli call for appending multiple reconstructed channels. I have tested this on the dataset from Hunter in this folder and it is working well. /hpc/projects/comp.micro/infected_cell_imaging/Image_preprocessing/Exp_2023_09_28_DENV_Fixed/
The time for processing has reduced to 1/3rd, which is great.
I too have the same question as @edyoshikun. I haven't figured out how to use the same for the dragonfly dataset. I have to reconstruct just the phase channel and append the fluorescence channels. I realize the code already exists in iohub for appending channels. Is there a way to do the same using cli, @ziw-liu ?

@edyoshikun
Copy link
Contributor

I also use the iohub code and it's relatively fast.

@talonchandler
Copy link
Collaborator Author

talonchandler commented Nov 1, 2023

Thanks for testing @Soorya19Pradeep!

@Soorya19Pradeep and I just discussed our next steps, and we agreed that:

  • there's no need to "copy channels" into a new zarr store, since the next downstream step (registration) can operate on two different .zarr stores.
  • this PR is ready for a code review---thanks @ziw-liu
  • when this merges I will release it as 0.4.2rc0.
  • Phenosight can depend on 0.4.2rc0, and we're on our way to a one-step installation.

recOrder/cli/utils.py Outdated Show resolved Hide resolved
@talonchandler talonchandler added this pull request to the merge queue Nov 1, 2023
Merged via the queue into main with commit 8f34a12 Nov 1, 2023
9 checks passed
@talonchandler talonchandler deleted the append-channels branch November 1, 2023 21:19
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.

4 participants