Improve sidecar parent checks

[larsoner]

Unrelated to this PR but #1508 already improved find_matching_sidecar to search the dataset root for sidecars that are missing from bids_path.directory. I feel like this could be extended to search other parent directories. For example, this scans.tsv is in the session directory.

Originally posted by @scott-huberty in #1556

[drammock]

Before #1508 it searched everything at or below sub-*/ (meaning it would already catch sidecars in ses-*/ directories). I think the problem for @scott-huberty in #1556 is that scans.tsv is not a sidecar file, it's a data summary file (https://bids-specification.readthedocs.io/en/stable/modality-agnostic-files/data-summary-files.html#scans-file)

[scott-huberty]

Before #1508 it searched everything at or below sub-*/ (meaning it would already catch sidecars in ses-*/ directories). I think the problem for @scott-huberty in #1556 is that scans.tsv is not a sidecar file, it's a data summary file (https://bids-specification.readthedocs.io/en/stable/modality-agnostic-files/data-summary-files.html#scans-file)

I guess you are right about that distinction. I wonder why my little trick in #1556 worked to 'make' find_matching_sidecar recognize the file. Might checkout the code before #1508 to see if it would recognize the scans file back then.

[drammock]

the trick worked because it returned a single, correct path? I'm not sure why you think it should find scans.tsv when datatype is set on the bidspath, since the scans.tsv file is at sub-*/ses-*/*_scans.tsv (not at sub-*/ses-*/eeg/*_scans.tsv).

[scott-huberty]

I'm not sure why you think it should find scans.tsv. when datatype is set on the bidspath, since the scans.tsv file is at sub-*/ses-*/*_scans.tsv (not at sub-*/ses-*/eeg/*_scans.tsv)

I thought that because

1. mne_bids.tests.test_path - test_find_matching_sidecar_at_root , finds a file named coordsystem.json(no datatype entity), from bids_path.find_matching_sidecar(suffix="coordsystem", ext=".json") - where bids_path.datatype == 'eeg'

2. To me It doesn't seem like it's a stretch to be able to be able to query this file, given that the scans.tsv file name was sub-01/ses-01/sub-01_ses-01_scans.tsv i.e. associated exactly with the subject and session.

3. I think that it is a QoL improvement. I needed to query scans.tsv in that test, and in the future others may want to be able to programmatically retrieve this file as well.

the trick worked because it returned a single, correct path?

Yes. without the .update(datatype=None), find_matching_sidecar would return 0 possible paths.

[drammock]

First, my point about sidecar vs data summary was I think incorrect; I thought that the sidecar naming conventions didn't apply to data summary files, but it turns out that the problem was dataset being specified.

Second, I think we've been misunderstanding each other? I've been saying basically "what you're seeing is what I expect based on how the code is written, why do you expect different?" and I think you've been saying "it's dumb that this code doesn't find the scans file, it obviously should from a user's POV".

I thought [it would work] because mne_bids.tests.test_path: test_find_matching_sidecar_at_root finds a file named coordsystem.json(no datatype entity), from bids_path.find_matching_sidecar(suffix="coordsystem", ext=".json") - where bids_path.datatype == 'eeg'

That's arguably a bug? The code for searching at dataset root is new and has its own code path; probably things should be refactored to unify behavior.

That said, I take your point that the way the function works (or did work before #1508) is not necessarily the optimum from a user / usability perspective, and it probably makes sense to loosen things up a bit. MNE-BIDS is supposed to make it comfortable/easy to wrangle the file paths, etc, and it's failing at that here.

I hate to say it, but it might make sense to redesign this part of the API. E.g., a new function (or multiple more specialized funcs?) that:

1. does JSON inheritance properly: e.g., can find JSON sidecars at multiple levels and merge them (but with option to select a specific level as needed)
2. is looser about path entities when there's an unambiguous "right" answer (like in this scans.tsv case)
3. probably other stuff I've forgotten