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

feat: Update thread to write to mda zarr #264

Closed
wants to merge 137 commits into from

Conversation

fdrgsp
Copy link
Contributor

@fdrgsp fdrgsp commented Jul 21, 2023

followup of #258.

In this PR I've updated the code with the suggestions in #258 + I've added a method (_process_remaining_frames) that is used to add to the zarr layer(s) any frames remaining in _deck after the acquisition is finished. Note that at the moment, since we don't have a proper writer in napari-micromanager this method is called by _on_mda_finished but once we have a writer, we can simply link this method to _io_thread _connect ("finished": self._process_remaining_frames).

To speed up the process it was also necessary to specify the chunk size when creating the zarr.

I did few test on our microscope and this way of handling the acquired images works great!

In addition I've added some fixes for the use of the current pymmcore-widgets (main git branch).
Test failing until new release of pymmcore-widgets.

closes #258.

@fdrgsp fdrgsp changed the title feat: Update thread to write to mda zarr [WIP] feat: Update thread to write to mda zarr Jul 22, 2023
@tlambert03
Copy link
Member

The private _sequence module definitely shouldn't be imported here. Let's talk about what you need to accomplish and make it public if there's no other way

@fdrgsp
Copy link
Contributor Author

fdrgsp commented Jul 23, 2023

The private _sequence module definitely shouldn't be imported here. Let's talk about what you need to accomplish and make it public if there's no other way

Sure :) I'm doing some tests with your sequencable PR and it's awesome!!! I pushed this commit to not loose the testing progress :)

@fdrgsp fdrgsp changed the title [WIP] feat: Update thread to write to mda zarr feat: Update thread to write to mda zarr Jul 27, 2023
@ianhi
Copy link
Contributor

ianhi commented Jul 27, 2023

In addition I've added some fixes for the use of the current pymmcore-widgets (main git branch).

@fdrgsp can you please make this a separate self contained PR. Wrapping it up in a big PR like this makes it difficult for me to work on other things (and led to me thinking this was an unfixed bug: #268)

@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Patch coverage: 65.21% and project coverage change: +2.28% 🎉

Comparison is base (a1e60c6) 75.24% compared to head (39855f2) 77.52%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #264      +/-   ##
==========================================
+ Coverage   75.24%   77.52%   +2.28%     
==========================================
  Files          15       15              
  Lines         820      801      -19     
==========================================
+ Hits          617      621       +4     
+ Misses        203      180      -23     
Files Changed Coverage Δ
...apari_micromanager/_gui_objects/_cam_roi_widget.py 0.00% <0.00%> (ø)
src/napari_micromanager/_saving.py 64.58% <ø> (+9.58%) ⬆️
src/napari_micromanager/_mda_handler.py 92.30% <85.10%> (+19.91%) ⬆️
..._micromanager/_gui_objects/_illumination_widget.py 100.00% <100.00%> (ø)
...rc/napari_micromanager/_gui_objects/_mda_widget.py 77.77% <100.00%> (+0.63%) ⬆️
src/napari_micromanager/_mda_meta.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fdrgsp
Copy link
Contributor Author

fdrgsp commented Aug 4, 2023

closing. Open a simpler version #272.

@fdrgsp fdrgsp closed this Aug 4, 2023
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.

None yet

3 participants