-
Notifications
You must be signed in to change notification settings - Fork 6
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 summary reportlets to HTML report #61
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #61 +/- ##
==========================================
+ Coverage 33.52% 33.75% +0.22%
==========================================
Files 56 56
Lines 6824 6852 +28
Branches 891 892 +1
==========================================
+ Hits 2288 2313 +25
Misses 4438 4438
- Partials 98 101 +3 ☔ View full report in Codecov by Sentry. |
The summary reportlets are being written to the |
@mattcieslak do you think QSIRecon should write out the recon spec? It seems useful. |
Definitely. It's at least as important as the config.toml file |
I add that in then |
Okay it's added and it looks good. |
# create a processing pipeline for the dwis in each session | ||
dwi_recon_wfs = {} | ||
dwi_individual_anatomical_wfs = {} | ||
recon_full_inputs = {} | ||
dwi_ingress_nodes = {} | ||
anat_ingress_nodes = {} | ||
print(dwi_recon_inputs) | ||
for dwi_input in dwi_recon_inputs: | ||
dwi_files = [dwi_input["bids_dwi_file"] for dwi_input in dwi_recon_inputs] | ||
for i_run, dwi_input in enumerate(dwi_recon_inputs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do the runs work here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no real idea how dwi_recon_inputs
organizes the input data, since I assumed that QSIRecon concatenated all of the DWI runs, but since it does collect potentially multiple files, I needed a step to merge them into a single list for the summary reportlet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh, in the case of dmri all the concatenation has happened in qsiprep. For qsirecon I think users will want to generate separate results for every dwi file that is in the inputs. If a researcher leaves the files split they most likely want to run recon workflows on them separately to test methoes (eg @ameliecr's project will do this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense
workflow.connect([(reduce_t1_preproc, summary, [("outlist", "t1w")])]) | ||
|
||
suffix_dirs = [] | ||
for qsirecon_suffix in config.workflow.qsirecon_suffixes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a great solution
Related to #13. Still missing run-wise anatomical and DWI summaries.
Changes proposed in this pull request