Repository: mne-tools/mne-bids
Pull request #1318 (2024-10-10 12:58:14)

Fix handling of README files with extensions
<!--
Thanks for contributing this pull request (PR).
If this is your first time, make sure to read
[CONTRIBUTING.md](https://github.com/mne-tools/mne-bids/blob/main/CONTRIBUTING.md)
-->

PR Description
--------------

According to [the BIDS specs](https://bids-specification.readthedocs.io/en/stable/glossary.html#readme-files), a README file can have no extension or one of the following: `.txt`, `.md`, `.rst.`

The current behavior of MNE-BIDS is to always assume an extensionless README to be present. A README file with one of the valid extensions is simply ignored, a new README file is written. This results in two README files being present in the BIDS root folder which violates the specs.

This PR fixes this by looking whether one of the valid extensions are present and uses the correct file.

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](https://github.com/mne-tools/mne-bids/blob/main/doc/whats_new.rst) is updated
- [ ] New contributors have been added to [CITATION.cff](https://github.com/mne-tools/mne-bids/blob/main/CITATION.cff)
- [ ] PR description includes phrase "closes <#issue-number>"


sappelhoff: Thanks! Please ping me once this is ready for review/merge.
thht: pinging @sappelhoff. all checks green. looking forward to your thoughts and input!
thht: > LGTM in general, thanks for the contribution.
> 
> However I'd also like the behavior to be that when multiple READMEs are discovered, we throw an error, instead of what this PR currently implements (selecting one README, and potentially ignoring the presence of the others)

Working on it right now ;-)
thht: As you can see in [my other reply](https://github.com/mne-tools/mne-bids/pull/1318#discussion_r1796813118), I added raising a `RuntimeError` when multiple READMEs are encounted including a test for this.

Patch:
diff --git a/doc/whats_new.rst b/doc/whats_new.rst
index ed1e235b..0bc77bb7 100644
--- a/doc/whats_new.rst
+++ b/doc/whats_new.rst
@@ -59,6 +59,7 @@ Detailed list of changes
 - Dealing with alphanumeric ``sub`` entity labels is now fixed for :func:`~mne_bids.write_raw_bids`, by `Aaron Earle-Richardson`_ (:gh:`1291`)
 - When processing subject_info data that MNE Python imports as numpy arrays with only one item, MNE-BIDS now unpacks these, resulting in a correct participants.tsv, by `Thomas Hartmann`_ (:gh:`1310`)
 - Fixed broken links in examples 7 and 8, by `William Turner`_ (:gh:`1316`)
+- All valid extensions for ``README`` files are now accepted. This prevents an extra ``README`` file being created, when one with a ``.txt``, ``.md``, or ``.rst`` extension is already present. By `Thomas Hartmann`_ (:gh:`1318`)
 
 ⚕️ Code health
 ^^^^^^^^^^^^^^
diff --git a/mne_bids/write.py b/mne_bids/write.py
index 500d300f..ec4ace2e 100644
--- a/mne_bids/write.py
+++ b/mne_bids/write.py
@@ -1955,7 +1955,18 @@ def write_raw_bids(
     )
 
     # For the remaining files, we can use BIDSPath to alter.
-    readme_fname = op.join(bids_path.root, "README")
+    readme_suffixes = ("", ".md", ".rst", ".txt")
+    found_readmes = sorted(
+        filter(lambda x: x.suffix in readme_suffixes, bids_path.root.glob("README*"))
+    )
+    if len(found_readmes) > 1:
+        raise RuntimeError(
+            "Multiple README files found in the BIDS root folder. "
+            "This violates the BIDS specifications. "
+            "Please ensure there is only one README file."
+        )
+    readme_fname = str((found_readmes or [bids_path.root / "README"])[0])
+
     participants_tsv_fname = op.join(bids_path.root, "participants.tsv")
     participants_json_fname = participants_tsv_fname.replace(".tsv", ".json")
 


Test patch:
diff --git a/mne_bids/tests/test_update.py b/mne_bids/tests/test_update.py
index 0a1b6cf8..868462e7 100644
--- a/mne_bids/tests/test_update.py
+++ b/mne_bids/tests/test_update.py
@@ -5,6 +5,7 @@
 
 import json
 import os.path as op
+import shutil
 from pathlib import Path
 
 import mne
@@ -38,7 +39,7 @@ bids_path = BIDSPath(
 )
 
 
-@pytest.fixture(scope="session")
+@pytest.fixture(scope="function")
 def _get_bids_test_dir(tmp_path_factory):
     """Return path to a written test BIDS dir."""
     bids_root = str(tmp_path_factory.mktemp("mnebids_utils_test_bids_ds"))
@@ -76,7 +77,7 @@ def _get_bids_test_dir(tmp_path_factory):
     return bids_root
 
 
-@pytest.fixture(scope="session")
+@pytest.fixture(scope="function")
 def _get_sidecar_json_update_file(_get_bids_test_dir):
     """Return path to a sidecar JSON updating file."""
     bids_root = _get_bids_test_dir
@@ -292,3 +293,57 @@ def test_update_anat_landmarks(tmp_path):
         assert np.allclose(
             mri_json["AnatomicalLandmarkCoordinates"][landmark], expected_coords
         )
+
+
+@testing.requires_testing_data
+@pytest.mark.parametrize("extension", [None, ".txt", ".rst", ".md"])
+def test_readme_conflicts(extension, _get_bids_test_dir):
+    """Test that exisiting README files are respected with any extension."""
+    bids_root = _get_bids_test_dir
+    assert Path(bids_root, "README").exists()
+    all_readmes = Path(bids_root).rglob("README*")
+    assert len(list(all_readmes)) == 1
+    if extension is not None:
+        shutil.move(Path(bids_root, "README"), Path(bids_root, f"README{extension}"))
+
+    bids_path = BIDSPath(
+        subject="02",
+        session=session_id,
+        run=1,
+        acquisition=acq,
+        task=task,
+        suffix="meg",
+        root=_get_bids_test_dir,
+    )
+
+    raw_fname = op.join(data_path, "MEG", "sample", "sample_audvis_trunc_raw.fif")
+    raw = mne.io.read_raw_fif(raw_fname)
+
+    write_raw_bids(raw, bids_path, overwrite=False)
+    all_readmes = Path(bids_root).rglob("README*")
+    assert len(list(all_readmes)) == 1
+    shutil.rmtree(bids_root)
+
+
+@testing.requires_testing_data
+def test_multiple_readmes_invalid(_get_bids_test_dir):
+    """Test that multiple README files are not allowed."""
+    bids_root = _get_bids_test_dir
+    assert Path(bids_root, "README").exists()
+    shutil.copy(Path(bids_root, "README"), Path(bids_root, "README.md"))
+    bids_path = BIDSPath(
+        subject="02",
+        session=session_id,
+        run=1,
+        acquisition=acq,
+        task=task,
+        suffix="meg",
+        root=_get_bids_test_dir,
+    )
+
+    raw_fname = op.join(data_path, "MEG", "sample", "sample_audvis_trunc_raw.fif")
+    raw = mne.io.read_raw_fif(raw_fname)
+
+    with pytest.raises(RuntimeError, match="Multiple README files found"):
+        write_raw_bids(raw, bids_path, overwrite=False)
+    shutil.rmtree(bids_root)