Skip to content

Commit 275eba1

Browse files
Point the download job at test_download.py, and trim the import test
download-test.yml ran `pytest ... download.py` from moabb/tests. That file does not exist -- it is test_download.py -- so pytest exited 4 and the job has failed every month since at least March while no @pytest.mark.download test ran anywhere in CI. The corrected path collects 537 tests. Also trims the new import side-effect test from 78 to 59 lines: the theme-is-still-applied assertion belongs with the leak assertion it qualifies.
1 parent e878029 commit 275eba1

3 files changed

Lines changed: 27 additions & 46 deletions

File tree

.github/workflows/download-test.yml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,5 +38,4 @@ jobs:
3838
- name: Run download tests
3939
run: |
4040
echo "Running tests"
41-
cd moabb/tests
42-
pytest -vv -s --tb=long --durations=0 --maxfail=5 --log-cli-level=INFO --dl-data download.py
41+
pytest -vv -s --tb=long --durations=0 --maxfail=5 --log-cli-level=INFO --dl-data moabb/tests/test_download.py

docs/source/whats_new.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ Requirements
3535

3636
Bugs
3737
~~~~
38+
- Point the monthly download job at ``moabb/tests/test_download.py``; it ran ``download.py``, a file that does not exist, so it collected nothing and has been failing every month while no ``@pytest.mark.download`` test executed anywhere in CI (by `Bruno Aristimunha`_).
3839
- Stop importing moabb from restyling the caller's matplotlib. ``moabb.analysis.plotting`` applies a seaborn theme to the global ``rcParams`` when it is imported, and three separate chains reached it eagerly, so ``import moabb.datasets`` alone changed ``font.family``, ``axes.grid`` and ``axes.spines.*`` for every figure the caller drew afterwards -- values matplotlib reads at axes creation, so they cannot be undone after the fact. The chain that mattered was indirect: :mod:`moabb.datasets.bids_interface` imports ``moabb.analysis.results`` for ``get_digest``, which ran ``moabb/analysis/__init__.py``, which imported ``plotting``. Those imports are now deferred to the functions that use them and to module-level ``__getattr__`` (:pep:`562`), so every public name still resolves -- ``dir()`` and Sphinx autodoc included -- and an explicit ``import moabb.analysis.plotting`` still applies the theme, leaving MOABB's own figures unchanged. ``import moabb`` also drops from about 2.6 s to 0.2 s (by `Bruno Aristimunha`_).
3940
- Type the seven non-EEG channels of :class:`moabb.datasets.BNCI2025_001` as ``misc``. ``read_raw_eeglab`` types anything it does not recognise as ``eeg``, so ``x``/``y``/``vx``/``vy``/``validity``/``targetPosX``/``targetPoxY`` -- the hand kinematics and the target position of the reaching task -- were picked as EEG by every paradigm and fed to classifiers as features, leaking the labels they encode. The declared ``n_channels`` (67) also disagreed with the dataset's own 60-electrode montage; it is now 71 with ``channel_types={"eeg": 60, "eog": 4, "misc": 7}``. Accuracies on this dataset will fall, which is the point (by `Bruno Aristimunha`_).
4041
- Give :class:`moabb.datasets.Rodrigues2017` the montage :gh:`700` announced but never shipped, the same omission as :class:`moabb.datasets.Cattan2019_PHMD` below: both share the 16-electrode setup, both spelled ``Fc5``/``Fc6`` -- the only two names ``standard_1020`` cannot resolve -- and neither loader called ``set_montage``. Its ``METADATA`` also declared ``standard_1010``, which is not a montage MNE can build (by `Bruno Aristimunha`_).
Lines changed: 25 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,78 +1,59 @@
1-
"""Importing moabb must not touch the caller's global state.
1+
"""Importing moabb must not restyle the caller's matplotlib.
22
3-
``moabb.analysis.plotting`` applies a seaborn theme to matplotlib's global
4-
rcParams at import time. That is fine for someone who asked for plotting, but
5-
it used to reach anyone who merely imported a dataset: ``moabb.datasets``
6-
imports ``moabb.analysis.results`` for ``get_digest``, which ran
7-
``moabb/analysis/__init__.py``, which imported ``plotting`` eagerly.
3+
``moabb.analysis.plotting`` themes the global rcParams at import time. That is
4+
fine for someone who asked for plotting, but it used to reach anyone who merely
5+
imported a dataset, via ``moabb.datasets.bids_interface`` -> ``analysis.results``.
86
"""
97

8+
import json
109
import subprocess
1110
import sys
1211

12+
import pytest
1313

14-
# Read at figure/axes creation, so a caller cannot undo them after the fact.
15-
_WATCHED = ("font.family", "axes.grid", "axes.spines.left", "axes.spines.top")
14+
15+
# Read at axes creation, so a caller cannot undo them afterwards.
16+
_WATCHED = ("font.family", "axes.grid", "axes.spines.left")
1617

1718
_PROBE = """
18-
import json, sys
19-
import matplotlib
19+
import json, sys, matplotlib
2020
matplotlib.use("Agg")
2121
import matplotlib.pyplot as plt
22-
23-
watched = {watched!r}
24-
before = {{k: repr(plt.rcParams[k]) for k in watched}}
22+
before = {{k: repr(plt.rcParams[k]) for k in {watched!r}}}
2523
import {module} # noqa: F401
26-
after = {{k: repr(plt.rcParams[k]) for k in watched}}
27-
json.dump({{"before": before, "after": after}}, sys.stdout)
24+
after = {{k: repr(plt.rcParams[k]) for k in {watched!r}}}
25+
json.dump([before, after], sys.stdout)
2826
"""
2927

3028

31-
def _rcparams_around_import(module):
32-
"""Import ``module`` in a fresh interpreter, reporting rcParams either side."""
29+
def _around_import(module):
30+
"""rcParams before and after importing ``module`` in a fresh interpreter."""
3331
out = subprocess.run(
3432
[sys.executable, "-c", _PROBE.format(module=module, watched=_WATCHED)],
3533
capture_output=True,
3634
text=True,
3735
check=True,
3836
)
39-
import json
40-
4137
return json.loads(out.stdout)
4238

4339

4440
def test_importing_moabb_does_not_restyle_matplotlib():
45-
for module in ("moabb", "moabb.datasets", "moabb.analysis", "moabb.paradigms"):
46-
seen = _rcparams_around_import(module)
47-
assert seen["before"] == seen["after"], (
48-
f"import {module} mutated global rcParams: "
49-
f"{seen['before']} -> {seen['after']}"
50-
)
51-
41+
for module in ("moabb", "moabb.datasets", "moabb.analysis"):
42+
before, after = _around_import(module)
43+
assert before == after, f"import {module} mutated rcParams: {before} -> {after}"
5244

53-
def test_plotting_still_applies_the_theme_when_asked_for():
54-
"""The theme must not be lost -- only stopped from leaking."""
55-
seen = _rcparams_around_import("moabb.analysis.plotting")
56-
assert seen["before"] != seen["after"]
45+
# The theme must be kept, not lost -- only stopped from leaking.
46+
before, after = _around_import("moabb.analysis.plotting")
47+
assert before != after
5748

5849

5950
def test_lazily_exported_names_still_resolve():
6051
import moabb
6152
import moabb.analysis
6253

63-
assert callable(moabb.benchmark)
64-
assert "benchmark" in dir(moabb)
54+
assert callable(moabb.benchmark) and "benchmark" in dir(moabb)
6555
for name in ("codecarbon_plot", "distribution_plot", "emissions_summary"):
66-
assert callable(getattr(moabb.analysis, name))
67-
assert name in dir(moabb.analysis)
68-
69-
70-
def test_unknown_attribute_still_raises_attributeerror():
71-
import moabb
56+
assert callable(getattr(moabb.analysis, name)) and name in dir(moabb.analysis)
7257

73-
try:
74-
moabb.definitely_not_a_real_attribute
75-
except AttributeError as exc:
76-
assert "definitely_not_a_real_attribute" in str(exc)
77-
else: # pragma: no cover
78-
raise AssertionError("expected AttributeError")
58+
with pytest.raises(AttributeError, match="definitely_not_real"):
59+
_ = moabb.definitely_not_real

0 commit comments

Comments
 (0)