Repository: mne-tools/mne-bids (python)

Issue:
Documentation Build Errors.
As seen in #1357 and #1355

the `docs-build` job is giving this error:

```
/home/circleci/project/mne_bids/path.py:docstring of mne_bids.BIDSPath.root:1: WARNING: py:class reference target not found: pathlib._local.Path [ref.class]
```

From local tests, my current hypothesis is that this has something to do with Python 3.13, which the CircleCI runner uses:

Because Locally, on python **3.10**, I don't get the error above, and the docstring of `BIDSPath.root` correctly hyperlinks to the python docs:

<img width="901" alt="Screenshot 2024-12-20 at 11 05 49 AM" src="https://github.com/user-attachments/assets/a423b256-5888-4401-a9c8-4f2f2fa59e7e" />


on Python **3.13** it looks like this ("Path" does not hyperlink):

<img width="800" alt="Screenshot 2024-12-20 at 11 10 54 AM" src="https://github.com/user-attachments/assets/67e7801f-8097-4ad9-ab04-534188a2e266" />


I don't think I have the skill to track down the source of that issue. But we can force the CircleCI runner that deploys `docs-build` to user a lower version of Python as a work around?

Pull request:
fix eager dtype conversion of value column
Follow-up to #1349, see https://github.com/mne-tools/mne-bids/pull/1349/files#r1885104885

- fixes #1358 

PR Description
--------------

Basically: use defensive programming when trying to dtype-convert the `value` column from `events.tsv`.  I was a bit worried that a column called `value` could easily exist in real datasets and not conform to the way that column gets written by MNE-BIDS, and it didn't take long for @larsoner to find one :)

The bug was already caught by a downstream test in MNE-BIDS-Pipeline, so I didn't bother adding a new test here, but LMK if you want me to add a simple one.

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>"


Patch:
diff --git a/doc/conf.py b/doc/conf.py
index e61a693d..d4cdaa00 100644
--- a/doc/conf.py
+++ b/doc/conf.py
@@ -114,6 +114,11 @@ release = version
 # This patterns also effect to html_static_path and html_extra_path
 exclude_patterns = ["auto_examples/index.rst", "_build", "Thumbs.db", ".DS_Store"]
 
+nitpick_ignore_regex = [
+    # needs https://github.com/sphinx-doc/sphinx/issues/13178
+    ("py:class", r".*pathlib\._local\.Path"),
+]
+
 # HTML options (e.g., theme)
 html_show_sourcelink = False
 html_copy_source = False
diff --git a/examples/convert_eeg_to_bids.py b/examples/convert_eeg_to_bids.py
index dbf0acdd..33e2cce4 100644
--- a/examples/convert_eeg_to_bids.py
+++ b/examples/convert_eeg_to_bids.py
@@ -59,7 +59,7 @@ from mne_bids.stats import count_events
 # to the "eyes closed" task.
 subject = 1
 run = 2
-eegbci.load_data(subject=subject, runs=run, update_path=True)
+eegbci.load_data(subjects=subject, runs=run, update_path=True)
 
 # %%
 # Let's see whether the data has been downloaded using a quick visualization
@@ -94,7 +94,7 @@ print_dir_tree(data_dir)
 # It prevents the data from being loaded and modified when converting to BIDS.
 
 # Load the data from "2 minutes eyes closed rest"
-edf_path = eegbci.load_data(subject=subject, runs=run)[0]
+edf_path = eegbci.load_data(subjects=subject, runs=run)[0]
 raw = mne.io.read_raw_edf(edf_path, preload=False)
 raw.info["line_freq"] = 50  # specify power line frequency as required by BIDS
 
diff --git a/examples/convert_group_studies.py b/examples/convert_group_studies.py
index 24dc4cee..7567c95a 100644
--- a/examples/convert_group_studies.py
+++ b/examples/convert_group_studies.py
@@ -50,7 +50,7 @@ runs = [
 run_map = dict(zip(runs, range(1, 4)))
 
 for subject_id in subject_ids:
-    eegbci.load_data(subject=subject_id, runs=runs, update_path=True)
+    eegbci.load_data(subjects=subject_id, runs=runs, update_path=True)
 
 # get path to MNE directory with the downloaded example data
 mne_data_dir = mne.get_config("MNE_DATASETS_EEGBCI_PATH")
@@ -81,7 +81,7 @@ raw_list = list()
 bids_list = list()
 for subject_id in subject_ids:
     for run in runs:
-        raw_fname = eegbci.load_data(subject=subject_id, runs=run)[0]
+        raw_fname = eegbci.load_data(subjects=subject_id, runs=run)[0]
         raw = mne.io.read_raw_edf(raw_fname)
         raw.info["line_freq"] = 50  # specify power line frequency
         raw_list.append(raw)
diff --git a/mne_bids/read.py b/mne_bids/read.py
index 8b1a6ead..149ca351 100644
--- a/mne_bids/read.py
+++ b/mne_bids/read.py
@@ -531,7 +531,8 @@ def _handle_events_reading(events_fname, raw):
     logger.info(f"Reading events from {events_fname}.")
     events_dict = _from_tsv(events_fname)
 
-    # drop events where onset is n/a
+    # drop events where onset is n/a; we can't annotate them and thus don't need entries
+    # for them in event_id either
     events_dict = _drop(events_dict, "n/a", "onset")
 
     # Get event descriptions. Use `trial_type` column if available.
@@ -547,9 +548,11 @@ def _handle_events_reading(events_fname, raw):
     # If we lack proper event descriptions, perhaps we have at least an event value?
     elif "value" in events_dict:
         trial_type_col_name = "value"
-    # Worst case: all events will become `n/a` and all values will be `1`
+    # Worst case: all events become `n/a` and all values become `1`
     else:
         trial_type_col_name = None
+        descrs = np.full(len(events_dict["onset"]), "n/a")
+        event_id = {descrs[0]: 1}
 
     if trial_type_col_name is not None:
         # Drop events unrelated to a trial type
@@ -569,26 +572,33 @@ def _handle_events_reading(events_fname, raw):
                         "Creating hierarchical event names."
                     )
                     for ii in idx:
-                        value = values[ii]
-                        value = "na" if value == "n/a" else value
+                        # strip `/` from `n/a` before incorporating into trial type name
+                        value = values[ii] if values[ii] != "n/a" else "na"
                         new_name = f"{trial_type}/{value}"
                         logger.info(f"    Renaming event: {trial_type} -> {new_name}")
                         trial_types[ii] = new_name
-            # drop rows where `value` is `n/a` & convert remaining `value` to int (only
-            # when making our `event_id` dict; `value = n/a` doesn't prevent annotation)
+            # make a copy with rows dropped where `value` is `n/a` (only for making our
+            # `event_id` dict; `value = n/a` doesn't prevent making annotations).
             culled = _drop(events_dict, "n/a", "value")
-            event_id = dict(
-                zip(culled[trial_type_col_name], np.asarray(culled["value"], dtype=int))
-            )
+            # Often (but not always!) the `value` column was written by MNE-BIDS and
+            # represents integer event IDs (as would be found in MNE-Python events
+            # arrays / event_id dicts). But in case not, let's be defensive:
+            culled_vals = culled["value"]
+            try:
+                culled_vals = np.asarray(culled_vals, dtype=float)
+            except ValueError:  # contained strings or complex numbers
+                pass
+            else:
+                try:
+                    culled_vals = culled_vals.astype(int)
+                except ValueError:  # numeric, but has some non-integer values
+                    pass
+            event_id = dict(zip(culled[trial_type_col_name], culled_vals))
         else:
             event_id = dict(zip(trial_types, np.arange(len(trial_types))))
         descrs = np.asarray(trial_types, dtype=str)
 
-    # Worst case: all events become `n/a` and all values become `1`
-    else:
-        descrs = np.full(len(events_dict["onset"]), "n/a")
-        event_id = {descrs[0]: 1}
-    # Deal with "n/a" strings before converting to float
+    # convert onsets & durations to floats ("n/a" onsets were already dropped)
     ons = np.asarray(events_dict["onset"], dtype=float)
     durs = np.array(
         [0 if du == "n/a" else du for du in events_dict["duration"]], dtype=float
diff --git a/pyproject.toml b/pyproject.toml
index b30bf0be..a4416593 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -1,6 +1,6 @@
 [build-system]
 build-backend = "hatchling.build"
-requires = ["hatch-vcs", "hatchling"]
+requires = ["hatch-vcs", "hatchling==1.26.3"]
 
 [project]
 authors = [{name = "The MNE-BIDS developers"}]


Test patch:
diff --git a/mne_bids/tests/test_read.py b/mne_bids/tests/test_read.py
index ef5b0077..70d56dbd 100644
--- a/mne_bids/tests/test_read.py
+++ b/mne_bids/tests/test_read.py
@@ -579,6 +579,16 @@ def test_handle_events_reading(tmp_path):
     ev_arr, ev_dict = mne.events_from_annotations(raw)
     assert event_id == ev_dict == {"n/a": 1}  # fallback behavior
 
+    # Test with only a (non-numeric) `value` column
+    events = {"onset": [10, 15], "duration": [1, 1], "value": ["A", "B"]}
+    events_fname = tmp_path / "bids6" / "sub-01_task-test_events.tsv"
+    events_fname.parent.mkdir()
+    _to_tsv(events, events_fname)
+    raw, event_id = _handle_events_reading(events_fname, raw)
+    # don't pass event_id to mne.events_from_annotatations; its values are strings
+    assert event_id == {"A": "A", "B": "B"}
+    assert raw.annotations.description.tolist() == ["A", "B"]
+
 
 @pytest.mark.filterwarnings(warning_str["channel_unit_changed"])
 @testing.requires_testing_data