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

Saving process #8

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Saving process #8

wants to merge 2 commits into from

Conversation

vigji
Copy link
Member

@vigji vigji commented Aug 17, 2020

Moving here code for the saving process from sashimi, and adding testing and cleanup in the process.
Changes so far:

  • moved email notification out of this process to separate module;

@codecov
Copy link

codecov bot commented Aug 17, 2020

Codecov Report

Merging #8 into master will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #8    +/-   ##
=======================================
  Coverage    0.00%   0.00%            
=======================================
  Files          24      26     +2     
  Lines        3179    3308   +129     
=======================================
- Misses       3179    3308   +129     
Impacted Files Coverage Δ
scopecuisine/__init__.py 0.00% <0.00%> (ø)
scopecuisine/email_notification.py 0.00% <0.00%> (ø)
scopecuisine/saving_process.py 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d4c7ace...2decb12. Read the comment docs.

@vilim
Copy link
Member

vilim commented Aug 17, 2020

To add Zarr and TIFF backends, I think it would make sense to add an extra (abstract) class: DataDestination, which would have a preallocate, add_plane, add_volume and finalize methods. We only need to think a bit about how to handle both 2p, volumentric and planar lightsheet cases (planar lightsheet probably just as 1-z-slice volumes)

@vigji
Copy link
Member Author

vigji commented Aug 17, 2020

Make sense, and planar lightsheet should definitively be just a 1z-slice volume.

@vigji
Copy link
Member Author

vigji commented Aug 20, 2020

@vilim following the current flow of the code, the saver should be initialised agnostic to xy plane size and then inferred it from the first stack. I think it would be cleaner to calculate them continuously with the (unsaved) frames that are streamed to the StackSaver and instantiate the DataDestination instance already with the proper size (in which case, the split dataset format could become something like a multiple inheritance DataDestination + SplitDataset object with methods for saving)

@vilim
Copy link
Member

vilim commented Aug 20, 2020

Well, the thing is that if the experiments gets manually stopped beforehand, you still want to save a vaild dataset, so the full dimensions cannot be know beforehand. I think it's fine that the DataDestination initializes the temporary storage (so the saver doesn't do that at all)

@vigji
Copy link
Member Author

vigji commented Aug 20, 2020

So in principle the full size should be known beforehand and "preallocated" (in the SplitDs attributes), but then the case of interruption should be handled?

@vilim
Copy link
Member

vilim commented Aug 20, 2020

This is one variant, the other is just to be agnostic to it. I don't care which it is as long as it works in all cases.

@vilim
Copy link
Member

vilim commented Sep 21, 2020

Note: I have added a notifier interface to master, so this needs to be changed a bit. (maybe adding Diego's picture-sending functionality from the 2p, and then making 2p rely on this, but that would be a separate PR).

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.

2 participants