From cd1e2f2f944bd5723237a203ada12aae0e3e955d Mon Sep 17 00:00:00 2001
From: Stefan Appelhoff <stefan.appelhoff@mailbox.org>
Date: Thu, 24 Oct 2024 19:14:04 +0200
Subject: [PATCH] properly deprecate and add test

---
 mne/datasets/eegbci/eegbci.py            | 34 +++++++++++++++++++-----
 mne/datasets/eegbci/tests/test_eegbci.py | 18 +++++++++++--
 2 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/mne/datasets/eegbci/eegbci.py b/mne/datasets/eegbci/eegbci.py
index a0089f412a8..f9fd1055513 100644
--- a/mne/datasets/eegbci/eegbci.py
+++ b/mne/datasets/eegbci/eegbci.py
@@ -9,7 +9,7 @@
 from os import path as op
 from pathlib import Path
 
-from ...utils import _url_to_local_path, logger, verbose
+from ...utils import _url_to_local_path, logger, verbose, warn
 from ..utils import _do_path_update, _downloader_params, _get_path, _log_time_size
 
 EEGMI_URL = "https://physionet.org/files/eegmmidb/1.0.0/"
@@ -94,9 +94,10 @@ def data_path(url, path=None, force_update=False, update_path=None, *, verbose=N
 
 @verbose
 def load_data(
-    subjects,
-    runs,
+    subjects=None,
+    runs=None,
     *,
+    subject=None,
     path=None,
     force_update=False,
     update_path=None,
@@ -116,6 +117,9 @@ def load_data(
         The subjects to use. Can be in the range of 1-109 (inclusive).
     runs : int | list of int
         The runs to use (see Notes for details).
+    subject : int
+        This parameter is deprecated and will be removed in mne version 1.9.
+        Please use ``subjects`` instead.
     path : None | path-like
         Location of where to look for the EEGBCI data. If ``None``, the environment
         variable or config parameter ``MNE_DATASETS_EEGBCI_PATH`` is used. If neither
@@ -154,11 +158,11 @@ def load_data(
     For example, one could do::
 
         >>> from mne.datasets import eegbci
-        >>> eegbci.load_data(1, [6, 10, 14], "~/datasets") # doctest:+SKIP
+        >>> eegbci.load_data([1, 2], [6, 10, 14], "~/datasets") # doctest:+SKIP
 
-    This would download runs 6, 10, and 14 (hand/foot motor imagery) runs from subject 1
-    in the EEGBCI dataset to "~/datasets" and prompt the user to store this path in the
-    config (if it does not already exist).
+    This would download runs 6, 10, and 14 (hand/foot motor imagery) runs from subjects
+    1 and 2 in the EEGBCI dataset to "~/datasets" and prompt the user to store this path
+    in the config (if it does not already exist).
 
     References
     ----------
@@ -166,6 +170,22 @@ def load_data(
     """
     import pooch
 
+    # XXX: Remove this with mne 1.9 ↓↓↓
+    # Also remove the subject parameter at that point.
+    # Also remove the `None` default for subjects and runs params at that point.
+    if subject is not None:
+        subjects = subject
+        warn(
+            "The ``subject`` parameter is deprecated and will be removed in version "
+            "1.9. Use the ``subjects`` parameter (note the `s`) to suppress this "
+            "warning.",
+            DeprecationWarning,
+        )
+        del subject
+    if subjects is None or runs is None:
+        raise ValueError("You must pass the parameters ``subjects`` and ``runs``.")
+    # ↑↑↑
+
     t0 = time.time()
 
     if not hasattr(subjects, "__iter__"):
diff --git a/mne/datasets/eegbci/tests/test_eegbci.py b/mne/datasets/eegbci/tests/test_eegbci.py
index 6ce58861c44..b63617a6919 100644
--- a/mne/datasets/eegbci/tests/test_eegbci.py
+++ b/mne/datasets/eegbci/tests/test_eegbci.py
@@ -2,12 +2,26 @@
 # License: BSD-3-Clause
 # Copyright the MNE-Python contributors.
 
+import pytest
+
 from mne.datasets import eegbci
 
 
 def test_eegbci_download(tmp_path, fake_retrieve):
     """Test Sleep Physionet URL handling."""
-    for subj in range(4):
-        fnames = eegbci.load_data(subj + 1, runs=[3], path=tmp_path, update_path=False)
+    subjects = range(1, 5)
+    for subj in subjects:
+        fnames = eegbci.load_data(subj, runs=[3], path=tmp_path, update_path=False)
         assert len(fnames) == 1, subj
     assert fake_retrieve.call_count == 4
+
+    # XXX: remove in version 1.9
+    with pytest.warns(DeprecationWarning, match="The ``subject``"):
+        fnames = eegbci.load_data(
+            subject=subjects, runs=[3], path=tmp_path, update_path=False
+        )
+        assert len(fnames) == 4
+
+    # XXX: remove in version 1.9
+    with pytest.raises(ValueError, match="You must pass the parameters"):
+        fnames = eegbci.load_data(path=tmp_path, update_path=False)