Splits are fetched by BIDSPath.fpath only when extension (and/or) suffix are missing

[dmalt]

Is there a reason for such behavior? I'm asking because I'm using BIDSPath as a kind of template tool for paths in my dataset. For instance, I like to use the same BIDSPath for writing a file and then loading it at the later processing steps.

The problem occurs when occasionally file sizes exceed the 2 Gb limit and have to be split. In this case, writing is still fine with split=None since MNE Python supports bids-style splits when saving raw data. Reading on the other hand becomes problematic because I have to either keep track of all the files which happen to have splits and update my templates separately, which is tedious or use the template BIDSPaths with suffix or extension unset, which I didn't realize until looking at the source code and which seem a little random to guess.

https://github.com/mne-tools/mne-bids/blob/74ac41a3f2035e4469f9930e725fe36f53f07146/mne_bids/path.py#L414-L446

So my question is can splits be handled irrespective of suffix and extension? And also can this fetching behavior of BIDSPath.fpath (and BIDSPath.__str__ for that matter) become more transparent? At least mentioning these rules in the docstrings would make a big difference I think.

Also maybe adding some flag like fetch_from_filesystem=True to make the behavior more explicit and controllable.

[dmalt]

Discussion in #692 is relevant I think

[hoechenberger]

And also can this fetching behavior of BIDSPath.fpath (and BIDSPath.str for that matter) become more transparent? At least mentioning these rules in the docstrings would make a big difference I think.

Also maybe adding some flag like fetch_from_filesystem=True to make the behavior more explicit and predictable.

My personal opinion is that we should stop doing these heuristics altogether, but… well, that's just me :)

[dmalt]

Couldn't agree more. Debugging this was a pain. Fetching splits as if there was a single file would be really useful though

[agramfort]

@dmalt can you profile a code snippet to demo the unnatural behaviour you observed? it's not clear to me as the split should be handled transparently.

[dmalt]

@agramfort this is what I mean

from pathlib import Path
from mne_bids import BIDSPath

root_path = Path("bids_root")
subj_path = root_path / "sub-01" / "meg"
subj_path.mkdir(exist_ok=True, parents=True)

bp_template = BIDSPath(
    subject="01", root="bids_root", suffix="meg", extension=".fif"
)
print(bp_template.fpath)
# prints bids_root/sub-01/meg/sub-01_meg.fif
# which is what I want to write raw data with raw.save

# now let's emulate situation when the raw file was saved in splits:
(subj_path / "sub-01_split-01_meg.fif").touch(exist_ok=True)
(subj_path / "sub-01_split-02_meg.fif").touch(exist_ok=True)

print(bp_template.fpath)
# still prints bids_root/sub-01/meg/sub-01_meg.fif
# because both suffix and extension are set
# Ideally I would like this to pick the first split

# At the same time
bp_template = BIDSPath(subject="01", root="bids_root", suffix="meg")
print(bp_template.fpath)
# outputs
# bids_root/sub-01/meg/sub-01_split-01_meg.fif
# But this template won't work for saving data since it's missing extension

# Bottom line:
# 1) Can't use the same template for reading and writing data
# 2) Can't guess the output of fpath without looking at source code
# 3) It seems strange that I need suffix and extension to pick splits

[agramfort]

if the files are split then

bp_template.fpath is indeed bids_root/sub-01/meg/sub-01_split-01_meg.fif

I don't understand why you say you need to update your template manually. mne_bids should as shown above direct you to split-01 file automatically.

i've had to deal with split files in a real dataset using https://github.com/mne-tools/mne-study-template/ and I had no problem after my last fix.

[dmalt]

No, I understand that it's possible to pick up splits of course. It's just that I had a hard time figuring out how to write the code properly for that because it's not mentioned in the docstring that BIDSPath.fpath would go to the filesystem to get the data. And my point is that it's not obvious why it would only work with one of extension and suffix unset.

[dmalt]

And I do care about setting both extension and suffix because I want to use the same BIDSPath for both reading and writing data. It feels logical and readable and requires very little code. I managed to make it work for my project, but the process was rough for me which is why I wanted to discuss it here.

[hoechenberger]

When writing, we return the (new? updated?) BIDSPath. Would using this one help in your case? (I actually haven't tried this out, so just leaving this here)

[dmalt]

You mean with write_raw_bids? Yes, looking at the docstring I think this might be an option. I need to test it out, thanks! I wrote my data with MNE Python. Since the writing is mostly for the derivatives, I don't care that much about all the metadata.

BTW, a kind of explanation about inferring the path there is exactly the thing that would be great to see in BIDSPath.fpath to avoid the surprises.

[hoechenberger]

Ok in this case I'm now confused and need to re-read the entire thread later tonight 😅

[dmalt]

I'm sorry I confused you :) Probably I'm not being clear enough, so I'll try to reformulate.

1. I don't like the fact that fpath getter does things behind the scenes and that its behavior changes depending on what files are present and which arguments were set in BIDSPath without saying anything about it in the docs and without any means to control it.
2. I also don't like what it does behind the scenes related to splits. In particular, I don't understand why it's required to unset either suffix or extension to pick up a path for a split file. It looks like the only requirement for fpath to pick the split should be that the fetched file can be uniquely identified in the filesystem.
3. I don't have a problem with making it work in my project now but it took some time to get there because of the first two points, so I wanted to discuss it with you to potentially improve fpath behavior and docs.

Does this make sense?

[agramfort]

ok got it ! yes this is clearly a bug !

this code block : https://github.com/mne-tools/mne-bids/blob/master/mne_bids/path.py#L423 is a mess. I edited this to support split files starting from @adam2392 's initial code and therefore I miss the initial underlying motivations for certain cases.

I very open to clarify what fpath should be doing depending on:

• if root is provided?
• if exension is provided? clearly guessing split files are only necessary if extension in [None, '.fif'] and suffix in [None, 'meg']

@adam2392 can you comment in to clarify what you expect fpath to return in different situations? It will be the basis of an improved docstring

and sorry @dmalt for the slow reaction and the time it took me to understand the issue...

[hoechenberger]

• clearly guessing split files are only necessary if extension in [None, '.fif'] and suffix in [None, 'meg']

suffix in [None, 'meg', 'eeg', 'ieeg'], no? iEEG datasets can be massive, and HD-EEG may also produce large files that may need to be split

[agramfort]

well fif is not part of standard beyond MEG but it's true that we use it in the study template ....

[hoechenberger]

Interesting thought!

[adam2392]

@adam2392 can you comment in to clarify what you expect fpath to return in different situations? It will be the basis of an improved docstring

and sorry @dmalt for the slow reaction and the time it took me to understand the issue...

I think the documentation could definitely be improved.

Rn:

• if root is not passed in, then fpath is just a relative path: e.g. sub-01/ses-01/<datatype>/sub-01_ses-01_task-01_run-01_<suffix>.<extension>
• If root is passed in, and a full filename is defined (e.g. suffix and extension), then it will just be like the above, but with root prepended: <root>/sub-01/ses-01/<datatype>/sub-01_ses-01_task-01_run-01_<suffix>.<extension>
• if root is passed in, then a full fpath will try to get inferred if one is missing the suffix and extension.

The third case is probably where it's potentially weird, and I also don't get what's going on w/ split files cuz I don't use them. So in this case, you might store data as BrainVision, and so you could theoretically leave out the extension='vhdr', and you can still use fpath to get the full filepath to the dataset (assuming you uniquely defined it with the other BIDS entities). Same w/ suffix.

[agramfort]

how about we disable fpath when root is None?

it will simplify things

[hoechenberger]

how about we disable fpath when root is None? it will simplify things

I think we currently simply treat root as . if it's not been passed. We could actually SET it to Path('.').absolute() upon instantiation if it is None

[agramfort]

defaulting to '.' yes would simplify things

[dmalt]

Thanks for your replies, everybody!

I would like to point out that BIDSPath.__str__ also relies on fpath and it looks really weird when there are no complaints from MNE-bids until one calls str(bids_path) and then it breaks because fpath tries to infer path and something goes wrong. Consider this. When BIDS_ROOT is a path to an existing BIDS dataset and there are multiple .fif files for sub-01, e.g. rest and task:

In [1]: bp = BIDSPath(root=BIDS_ROOT, subject="01")
In [2]: str(bp)  # simply checking what my BIDSPath looks like                                                                              
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
<ipython-input-9-810b7d4e3b46> in <module>
----> 1 str(bp)

~/miniconda3/envs/mne_bids_latest/lib/python3.8/site-packages/mne_bids/path.py in __str__(self)
    346     def __str__(self):
    347         """Return the string representation of the path."""
--> 348         return str(self.fpath)
    349 
    350     def __repr__(self):

~/miniconda3/envs/mne_bids_latest/lib/python3.8/site-packages/mne_bids/path.py in fpath(self)
    463                            'BIDS dataset. Please run the BIDS validator on '
    464                            'your data.')
--> 465                     raise RuntimeError(msg)
    466                 else:
    467                     bids_fpath = matching_paths[0]

RuntimeError: Found more than one matching data file for the requested recording. Cannot proceed due to the ambiguity. This is likely a problem with your BIDS dataset. Please run the BIDS validator on your data.

Don't you think it might be reasonable to make fpath simply assemble path from the given entities and have a separate function for inferring paths from the dataset? For instance, there's BIDSPath.match() which already sort of does this.

[adam2392]

^ I'm in favor of that.

• simplify fpath/str
• perhaps a function to try to "infer" raw dataset path (similar to match, but only tries to get the raw EEg/meg/ieeg dataset)

[dmalt]

Yes, this sounds great

[agramfort]

see https://github.com/mne-tools/mne-bids/pull/721 to start the ball rolling