jmtyszka / bidskit

Utility functions for working with DICOM and BIDS neuroimaging data
MIT License
61 stars 41 forks source link

--bind-fmaps not yet set up to work with --no-sessions #98

Closed sarahoh closed 2 years ago

sarahoh commented 2 years ago

Calling bidskit with --bind-fmaps flag does not populate the IntendedFor field in the fmap/*epi.json files when used together with --no-sessions flag.

It seems that the bind_fmaps function in translate.py expects session directories even when --no-sessions is being used (see screenshot below).

image (1)

paola-g commented 2 years ago

Hi, I encountered the same issue and came up with a (quite rudimental) solution here: https://github.com/paola-g/bidskit/blob/master/bidskit/fmaps.py

If you have any suggestions on how to make it more elegant/robust I could open a pull request.

jmtyszka commented 2 years ago

Hi @paola-g! Your solution looks eminently practical and if it works for most cases, all the better. The only thing I'd change is to pass the --no-sessions flag down to this function rather than check for zero length sessions arrays and maybe put the common code into a function. Go ahead and open a pull request and we can finish this up for @sarahoh

jmtyszka commented 2 years ago

Hold off on the PR for now. I coded up a solution that uses the no-sessions flag and uses a more general approach to the subjects/sessions directories in binds_fmaps(). Testing it now and will commit if it seems to work for a session-less structure.

jmtyszka commented 2 years ago

Just pushed a tested solution for the session-less fmap binding (v2022.5.18). Let me know if it does/doesn't work as intended (or IntendedFor :)) with your data @paola-g and @sarahoh. If all's well I'll push this version to pypi. Thanks @paola-g for offering a fix! Ended up being a little more involved because the IntendedFor field needs to account for the lack of sessions.

paola-g commented 2 years ago

Thanks Mike, you were faster than me with the implementation! I will test on my data and report back.

paola-g commented 2 years ago

Just tested, seems to work fine!