fmriprep
fmriprep copied to clipboard
nipreps
ENH : separate deep from shallow WM+CSF in the carpetplot
This PR implements the proposition of @effigies (https://github.com/nipreps/niworkflows/pull/651#issuecomment-961103549) to separate the WM+CSF compartement in the carpetplot into the shallow WM+CSF and the deep WM+CSF corresponding to the aCompCor mask
Hello @celprov! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:
Comment last updated at 2022-05-27 17:18:00 UTC
Not sure if this is an artifact of our shorter test datasets, but we've lost most of the sections of our carpetplot:
https://output.circle-artifacts.com/output/job/51b32812-2f87-4605-802c-4f98d4973703/artifacts/0/tmp/ds054/derivatives/fmriprep/sub-100185_noerror.html#datatype-figures_desc-summary_run-1_suffix-bold_task-machinegame
Either way, if we figure this out before the full release, I'm happy to include it, but for now I'll release the RC without this.
Hi @effigies, thanks for spotting this issue. I'll try to work on it after I come back from holiday. When is the full release planned for ?
@effigies oh ok very soon. I don't think I will make it then, but I will give it a go if time allows.
Not sure if this is an artifact of our shorter test datasets, but we've lost most of the sections of our carpetplot
To be clear, this is not currently happening on master, right? (i.e., it's a problem of this PR)
Not sure if this is an artifact of our shorter test datasets, but we've lost most of the sections of our carpetplot
To be clear, this is not currently happening on master, right? (i.e., it's a problem of this PR)
Correct.
@effigies Good timing! I was precisely thinking about working on this problem this week.
@celprov Looks like my previous suggestion used bool instead of np.bool_. Latest commit resolves the plotting issue.
Will merge on tests passing.
