MRG: Add "description" entity to BIDSPath

[hoechenberger]

Fixes #1049
Fixes #449

It seems to be working already:

# %%
import mne
import mne_bids

sample_dir = mne.datasets.sample.data_path()
raw_path = sample_dir / 'MEG' / 'sample' / 'sample_audvis_raw.fif'
raw = mne.io.read_raw_fif(raw_path, preload=False)

bp = mne_bids.BIDSPath(
    subject='foo',
    task='bar',
    description='baz',
    suffix='meg',
    extension='.fif',
    datatype='meg',
    root='/tmp',
)
bp_written = mne_bids.write_raw_bids(
    raw=raw,
    bids_path=bp,
    overwrite=True
)
mne_bids.read_raw_bids(bp_written)

@sappelhoff It's unclear to me where exactly to inject the desc entity in the BIDS path. For now, I'm putting it at the very end, before split and suffix. I couldn't find anything in the BIDS spec to help here.

@agramfort Please feel free to test and also to finalize ;)

Merge checklist

Maintainer, please confirm the following before merging.
If applicable:

• All comments are resolved
• This is not your own PR
• All CIs are happy
• PR title starts with [MRG]
• whats_new.rst is updated
• New contributors have been added to CITATION.cff
• PR description includes phrase "closes <#issue-number>"

[hoechenberger]

dict insertion order is preserved in Python 3.7+, so no need to have an OrderedDict anymore

[hoechenberger]

Needs to be added to get_entities_from_fname() (???) too

[agramfort]

I would say yes to be consistent

…

Message ID: ***@***.***>

[hoechenberger]

Yep absolutely, I just wasn't sure about the exact name of that function :)

[hoechenberger]

Doc build is broken due to recent changes upstream in MNE.

[agramfort]

@alexrockhill can you have a look the doc build issue? It’s related to seeg example 🙏

[sappelhoff]

@sappelhoff It's unclear to me where exactly to inject the desc entity in the BIDS path. For now, I'm putting it at the very end, before split and suffix. I couldn't find anything in the BIDS spec to help here.

see: https://github.com/bids-standard/bids-specification/blob/b3afa2ada5faaebeef9c48e6074a6fa5174289a3/src/schema/rules/entities.yaml#L1-L31

[hoechenberger]

Wow, how did you even find that 😅 but perfect

[hoechenberger]

Actually no it's not good. Why is split not last? Having it not be last doesn't make any sense to me.

[hoechenberger]

Failing tests all seem to be due to new set_montage() behavior upstream.

[codecov]

Codecov Report

Merging #1057 (fdeb5ef) into main (7eb37ef) will increase coverage by 1.21%.
The diff coverage is 90.90%.

@@            Coverage Diff             @@
##             main    #1057      +/-   ##
==========================================
+ Coverage   93.95%   95.17%   +1.21%     
==========================================
  Files          25       25              
  Lines        3822     3831       +9     
==========================================
+ Hits         3591     3646      +55     
+ Misses        231      185      -46     

Impacted Files	Coverage Δ
mne_bids/config.py	97.61% <ø> (ø)
mne_bids/path.py	97.56% <90.90%> (-0.11%)	⬇️
mne_bids/utils.py	95.74% <0.00%> (+0.53%)	⬆️
mne_bids/write.py	97.06% <0.00%> (+1.82%)	⬆️
mne_bids/dig.py	98.27% <0.00%> (+9.65%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[adam2392]

Just a loose thought...

Is it worth differentiating "raw" entities and "derivative" entities? E.g. via a parameter passed to entities?

[hoechenberger]

I was thinking about this too at least for writing, but I'd say we can always add this later on... for now we had a specific use case where @agramfort couldnt read some OpenNeuro data with MNE-BIDS, and this PR fixes this :)

[alexrockhill]

@alexrockhill can you have a look the doc build issue? It’s related to seeg example pray

Looks like it's passing now, happy to review though.

[agramfort]

@sappelhoff merge if happy

🙏 @hoechenberger

[sappelhoff]

Thanks Richard!