Drop functions to read sidecar JSONs #760
Conversation
tsalo
added
the
refactor
|
I haven't removed |
| @@ -115,7 +114,9 @@ def init_merge_and_denoise_wf( | |||
| bids_dwi_files=raw_dwi_files, | |||
| b0_threshold=config.workflow.b0_threshold, | |||
| harmonize_b0_intensities=not config.workflow.no_b0_harmonization, | |||
| scan_metadata={scan: get_metadata_for_nifti(scan) for scan in raw_dwi_files}, | |||
| scan_metadata={ | |||
| scan: config.execution.layout.get_metadata(scan) for scan in raw_dwi_files | |||
There was a problem hiding this comment.
The only issue is that read_nifti_sidecar is still used in places. It looks like there are a few interfaces/functions (get_distortion_grouping and get_best_b0_topup_inputs_from) that use read_nifti_sidecar and do some complicated stuff, so I didn't try to remove it.
|
The readthedocs error is very strange, I don't think this PR changes anything in extras_require |
|
No, but I'm seeing the same failure in #766. I think it might be an issue with DiPy. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #760 +/- ##
==========================================
+ Coverage 29.76% 29.84% +0.07%
==========================================
Files 97 97
Lines 14586 14604 +18
Branches 1887 1885 -2
==========================================
+ Hits 4342 4359 +17
+ Misses 10118 10117 -1
- Partials 126 128 +2 ☔ View full report in Codecov by Sentry. |
|
I think this will work, but I'm wondering if we should hold off on major refactors until we add a couple more CI tests (like for GRE fieldmaps). Wdyt? |
|
I'm happy to wait on it. |
Closes #759.
Changes proposed in this pull request
qsiprep.interfaces.bids.ReadSidecarJSON(this was actually unused, as the interface that was used ininit_phdiff_wfwas the one imported from niworkflows).qsiprep.interfaces.bids.get_metadata_for_niftiin favor ofconfig.execution.layout.get_metadata.ReadSidecarJSONcalls in favor ofconfig.execution.layout.get_metadata.phase_metainput toinit_phdiff_wf. This actually matches the SDCFlows workflow a bit better as an added bonus.