fiuneuro / cis-bidsify

Tools for Singularity- and heudiconv-based BIDSification and anonymization of MRI data optimized for the FIU HPC.
Apache License 2.0
0 stars 2 forks source link

[REF] Convert to pure Python #13

Closed tsalo closed 4 years ago

tsalo commented 4 years ago

Closes #14, closes #17, closes #16. The end result of this PR should be that cis-bidsify is written completely in Python.

Still to-do:

  1. ~Run bids-validator.~
  2. Check minipath (subject- and session-specific output file from heudiconv).
tsalo commented 4 years ago

@adamkimbler I noticed that we don't have a Python equivalent to this if statement. Do you know if it's necessary?

akimbler commented 4 years ago

@tsalo I hate shell scripting. Is that just checking for existence of the path? If so that's easily done with pathlib. If not I have no idea what its doing.

tsalo commented 4 years ago

It checks for existence of the sub-XXX or sub-XXX/ses-YYY folder in the output directory after heudiconv is run. If the folder doesn't exist, then it skips the additional steps (defacing, metadata cleanup, etc.) and writes out to the validator text file that the conversion failed. I wonder, though, if that folder will still be created even if heudiconv fails (in which case, the if/else statement wouldn't catch most failures). I think heudiconv creates the folder after all files are indexed, but before anything is converted. Do you have any thoughts on where failures tend to occur and if the if statement is important?

akimbler commented 4 years ago

Yeah, I think we need a better check that isn't contingent on those folders existing. I need to fiddle around with things to check. We're executing heudiconv with subprocess right? Wouldn't it be better to check against the exit code of that process for completion?

akimbler commented 4 years ago

@tsalo one other thing before merging this PR. I think the traditional way of import from pathlib is from pathlib import Path since everything in the package is based on working with the Path object. We should probably change to from pathlib import Path and replace all instances of pathlib.Path with just Path, which should help minimize some line length

tsalo commented 4 years ago

Merging now.