Zero out parcels with <50% coverage #757
Conversation
tsalo
added
enhancement
xcp_d/tests/test_conn.py
Outdated
| ) | ||
| atlas_file = os.path.join( | ||
| connectivity_wf.base_dir, | ||
| "connectivity_wf/warp_atlases_to_bold_space/mapflow/_warp_atlases_to_bold_space3", |
There was a problem hiding this comment.
is it safe to look into hardcoded mapnode directories?
There was a problem hiding this comment.
It's definitely fragile, and any changes to the workflows will inevitably break the tests. The old tests used wildcards to glob through everything, but it was hard to interpret them. I'd love to somehow just look in the workflow object, but AFAICT the inputs and outputs of the different nodes within the workflow can't be accessed like that.
There was a problem hiding this comment.
I think if you have access to the workflow object you can use get_node() and then access the input/outputs. Not critical for this PR, but could be a nice addition to the pytest lifestyle
There was a problem hiding this comment.
That's AWESOME! I had no idea. I'll definitely clean up the tests using this trick at some point.
Closes #756.
One thing I'm noticing is that
-cifti-correlatehas ones in the diagonal, even for parcels with a mean of zero throughout the time series (in numpy these parcels result in a NaN in the diagonal).Changes proposed in this pull request
CiftiZerosToNaNsand complicated coverage-measuring steps withCiftiCreateDenseFromTemplate(to map the atlases onto the data file) andCiftiPrepareForParcellation(to handle the parcel coverage issue).