New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Relax requirement for input BIDS data structure for first level GLM #3567
Comments
I just want to ping in my concern about embarking in this direction : AFAICT, BIDS support in Nilearn is not designed to be comprehensive. We've also historically been very hesitant to support "newer" methods or less common use cases, which I argue this would fall under. |
The thing is that this is independent of the code and completely dependent on how people organize their data. Even if we had a lightning-fast SQL-free pybids and that we decided to rely on this, the moment that the derivatives are not nested in your BIDS dataset, you are going to have to "index" 2 datasets. Also I am not so sure the nested derivatives is actually the common use case. From a grep on datalad openneuro superdataset I can find:
~/datalad/datasets.datalad.org/openneuro:
╰─⠠⠵ ls -l | wc -l
757
~/datalad/datasets.datalad.org/openneuro:
╰─⠠⠵ tree -L 2 | grep derivatives | wc -l
185
~/datalad/datasets.datalad.org/openneuro:
╰─⠠⠵ tree */derivatives/ -d -L 1 | grep fmriprep | wc -l
21 |
I understand that point, but I just want to emphasize that I don't think nilearn is the place to support many different data organizations. I may very well be in the minority, but I'm just hesitant to prioritize supporting multiple file organizations. In my mind, Nilearn isn't targeting maximal BIDS compliance, just allowing users to access a few common BIDS patterns. Again, this is my personal perspective, though. |
We can also leave that issue open and wait till we get users asking for it to be implemented. I don't think that it would launch us on a slippery slope towards scope creep but given that no one has asked for it so far I do not believe it is urgent either. |
I think I agree with @emdupre that having a comprehensive and high-quality BIDS interface is a great goal, but maybe not for Nilearn. To be discussed. |
PyBIDS is going to merge ANCP-BIDS to their code base: |
Thanks, @htwangtw ! Last I heard this wasn't yet a done deal (not sure if you've heard otherwise ?), but I hope it's going to have more updates soon 🤞 |
After thinking about it for it, there's no harm if we integrate ancp-bids is the two merges in the end |
Chris has now opened an issue proposing one path forward for ancp-bids / PyBIDS : bids-standard/pybids#989 From out-of-band discussions with him, it might make sense for us to think about the API we need (and weigh in there), so we can design something that's neither ancp-bids nor pybids specific ! |
yes definitely a good place to make "feature request" |
closing for now let's wait till we have users asking for it to think about implementing |
FYI just checked and it seems that this issue is solved by using relative path. Nilearn by default expects derivatives data to be nested in the BIDS dataset. But when that's not the case:
One can point to the derivatives by using relative paths: models, imgs, events, confounds = first_level_from_bids(
dataset_path="/home/remi/github/nilearn/nilearn/tmp/bids_dataset",
task_label=tasks[1],
space_label="MNI",
img_filters=[("desc", "preproc")],
slice_time_ref=None,
derivatives_folder="../derivatives"
) |
Currently the
first_level_from_bids
expects the derivatives folder to be nested within the bids raw dataset. Although this is a common layout, this is by no mean the only one.Benefits to the change
More flexibility of the allowed input layouts for users.
The implementation should not be too hard but may require some care should be taken in case 2 to not search files in the sourcedata folder (which is actually a good general rule anyway).
The text was updated successfully, but these errors were encountered: