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

Initialize calibration values when loading from metadata files #184

Merged
merged 5 commits into from
Sep 20, 2022

Conversation

ziw-liu
Copy link
Contributor

@ziw-liu ziw-liu commented Sep 20, 2022

Previously loading calibration from a metadata file does not initialize LC values in the QLIPP_Calibration object created by the load_calibration thread worker, which causes an error when trying to capture background afterwards. This PR addresses the error by setting the required values, although the necessity of fully initializing the QLIPP_Calibration object when loading from metadata files is still up to debate.

@ziw-liu ziw-liu added the bug Something isn't working label Sep 20, 2022
@ziw-liu
Copy link
Contributor Author

ziw-liu commented Sep 20, 2022

I'm not currently labelling this issue as automatically closing #180 upon merging. Feel free to change it as needed.

@ziw-liu ziw-liu added this to the 0.2.0 milestone Sep 20, 2022
@deprecated-napari-hub-preview-bot

Preview page for your plugin is ready here:
https://preview.napari-hub.org/mehta-lab/recOrder/184
Created: 2022-09-20T00:47:08.439223

@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2022

Codecov Report

Merging #184 (e423fa7) into main (c70b2ed) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #184      +/-   ##
==========================================
- Coverage   11.20%   11.17%   -0.03%     
==========================================
  Files          45       45              
  Lines        5987     6002      +15     
==========================================
  Hits          671      671              
- Misses       5316     5331      +15     
Impacted Files Coverage Δ
recOrder/plugin/workers/calibration_workers.py 0.00% <0.00%> (ø)
recOrder/io/utils.py 0.00% <0.00%> (ø)
recOrder/plugin/widget/main_widget.py 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ziw-liu ziw-liu marked this pull request as ready for review September 20, 2022 16:14
Copy link
Collaborator

@talonchandler talonchandler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ziw-liu!

I just tested, and everything is working as I expect. I will link #180 since this fixes that issue.

My only question is about the new import statement. Is it necessary?

As we discussed, the data flow in the calibration metadata files is a bit wonky right now. We'll design this more carefully and clean this up for 1.0.0.

Thanks!

@ziw-liu ziw-liu merged commit 2a3e0d4 into main Sep 20, 2022
@ziw-liu ziw-liu deleted the fix-load-calibration-attr branch September 20, 2022 21:57
ziw-liu added a commit that referenced this pull request Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] "Load calibration" followed by "Capture Background" gives error
3 participants