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

AFID QC Feature #24

Merged
merged 3 commits into from
Mar 12, 2025
Merged

AFID QC Feature #24

merged 3 commits into from
Mar 12, 2025

Conversation

ataha24
Copy link
Contributor

@ataha24 ataha24 commented Mar 7, 2025

🚀 PR: AFID QC Feature

🧠 Overview
This PR enables a QC feature which takes as input the AFIDs predicted via AutoAFIDs and generates an interactive .html for the user. The user can toggle different MRI slices and visualize the placements at different anatomical slices with a red crosshair showing the location of the AFID of interest. Furthrmore, the user can pan between the template defined placement and the subject placement to enable for a more informed QC process.

🔍 How It Works
New functions which slice MRI at various levels around the coordinates of interest.

📂 Outputs of stereotaxy.smk
.html file: Contains the predicted coordinates and on native space raw MRI image.

🔧 Changes to the Codebase

utils.py
Added new helper functions for *.html file generation, and various helper function.

fidqc.smk
New rule to take the input image of the workflow and corresponding .fcsv file predictions.

📁 Resource Additions
None.

📚 Documentation Updates
Instructions on how to use the fidqc feature in config file.

⚠️ Known Issues
None.

Base automatically changed from djay/docker to main March 7, 2025 10:48
@Dhananjhay
Copy link
Contributor

Amazing work here @ataha24! I was successfully able to run this feature and it looks great! I've resolved the merge conflicts and updated the dag, but would need @mackenziesnyder to work her magic once again to create the final version of the dag.

@mackenziesnyder
Copy link
Contributor

image This worked well for me!! One thing I noticed (pictured above) is that everything in the qc is labelled Landmark 1-32. In the lab meeting I think I remember a comment about naming them by the landmark name like AC, PC - I just wanted to make sure this naming was intentional for this PR in case the most recent changes aren't pushed yet.


html_content += f"""
<div class="container">
<h2>Landmark: {label}</h2>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh great catch @mackenziesnyder -- we probably just need to index the dictionary to print the name here.

(i.e., {afids_labels[label]}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated code to now call the short form AFID. I think they probably would mean the same to a new user (i.e., they may not be familiar with the namings). But, I guess short form may still be a bit more informative than just a number?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes just tested it out, I agree that makes sense to me

@ataha24
Copy link
Contributor Author

ataha24 commented Mar 12, 2025

@Dhananjhay & @mackenziesnyder looks like this one is good 2 go!

@Dhananjhay
Copy link
Contributor

Great work guys! Merging it now.

@Dhananjhay Dhananjhay merged commit 80da7cd into main Mar 12, 2025
@Dhananjhay Dhananjhay deleted the afidqc-feature branch March 12, 2025 15:58
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.

3 participants