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

Add ability to override run callbacks to process per FOV #313

Merged
merged 35 commits into from
Mar 13, 2023

Conversation

alex-l-kong
Copy link
Contributor

@alex-l-kong alex-l-kong commented Jan 31, 2023

What is the purpose of this PR?

Closes #311. Closes #306. Closes #311

How did you implement your changes

See design doc in #311.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@alex-l-kong alex-l-kong changed the title Inter callback Add ability to override run callbacks to process per FOV Jan 31, 2023
@alex-l-kong
Copy link
Contributor Author

@srivarra all up and running now! The failing tests are from the old MIBItracker API code, we don't have to worry about that for this PR.

@ngreenwald
Copy link
Member

@alex-l-kong and @srivarra, will you guys take a look through the whole thing to confirm you guys are both happy with the final result?

toffy/mph_comp.py Outdated Show resolved Hide resolved
@alex-l-kong
Copy link
Contributor Author

@srivarra and I reviewed, looks good on our end.

@ngreenwald ngreenwald merged commit e654963 into main Mar 13, 2023
@ngreenwald ngreenwald deleted the inter_callback branch March 13, 2023 23:43
Copy link
Contributor

@camisowers camisowers left a comment

Choose a reason for hiding this comment

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

Just one minor fyi comment about the logic of compute_qc_metrics_direct.

@@ -264,14 +266,11 @@ def sort_bin_file_fovs(fovs, suffix_ignore=None):
)


def compute_qc_metrics(bin_file_path, extracted_imgs_path, fov_name,
def compute_qc_metrics(extracted_imgs_path, fov_name,
Copy link
Contributor

@camisowers camisowers Mar 13, 2023

Choose a reason for hiding this comment

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

Note that the generate_qc FOV callback actually calls compute_qc_metric_direct itself, not this function. I think this was leftover from when the function above used the bin files for calculation, so Adam made a "direct" function to skip that re-extraction while running watcher.

@camisowers camisowers added the enhancement New feature or request label Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants