From bab5d115ef62afa210520573524eebc41895a67d Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sun, 4 Oct 2026 09:25:30 -0400 Subject: [PATCH 1/4] fix(sync): survive a broken optional install, accept the metric aliases An optional package that is installed but fails to import (torch, numba, cupy, Metal) no longer makes hypyp fail to import: the backend is disabled with a warning that gives the original error. get_metric accepts envelope_corr, pow_corr and imaginary_coh as compute_sync does. The hints for a missing numba or torch give the pip extras instead of a poetry command that no longer exists. Co-Authored-By: Claude Fable 5.1 --- hypyp/sync/__init__.py | 12 +++++- hypyp/sync/base.py | 22 +++++++++- hypyp/sync/kernels/__init__.py | 18 +++++++++ tests/test_sync.py | 74 ++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 3 deletions(-) diff --git a/hypyp/sync/__init__.py b/hypyp/sync/__init__.py index 10a1321..87a533b 100644 --- a/hypyp/sync/__init__.py +++ b/hypyp/sync/__init__.py @@ -74,6 +74,13 @@ "powcorr": PowCorr, } +# Alternative names of the metrics, accepted by get_metric and compute_sync +METRIC_ALIASES = { + "envelope_corr": "envcorr", + "pow_corr": "powcorr", + "imaginary_coh": "imcoh", +} + __all__ = [ # Base classes and utilities "BaseMetric", @@ -94,6 +101,7 @@ "PowCorr", # Registry "METRICS", + "METRIC_ALIASES", "get_metric", ] @@ -108,7 +116,8 @@ def get_metric( ---------- mode : str Name of the connectivity metric. One of: 'plv', 'ccorr', 'accorr', - 'coh', 'imcoh', 'pli', 'wpli', 'envcorr', 'powcorr'. + 'coh', 'imcoh', 'pli', 'wpli', 'envcorr', 'powcorr'. The aliases + 'imaginary_coh', 'envelope_corr' and 'pow_corr' are accepted too. optimization : str, optional Optimization strategy. Options: None, 'auto', 'numba', 'torch', 'metal', 'cuda_kernel'. See BaseMetric for fallback behavior. @@ -134,6 +143,7 @@ def get_metric( ... priority=['numba', 'metal']) # custom priority """ mode_lower = mode.lower() + mode_lower = METRIC_ALIASES.get(mode_lower, mode_lower) if mode_lower not in METRICS: available = ", ".join(METRICS.keys()) raise ValueError(f"Unknown metric mode '{mode}'. Available: {available}") diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index 68cb1a3..ef82b57 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -60,6 +60,17 @@ TORCH_AVAILABLE = False MPS_AVAILABLE = False CUDA_AVAILABLE = False +except Exception as exc: + # Installed but unusable (a missing shared library, for instance): + # disable the backend instead of making `import hypyp` fail + warnings.warn( + "torch is installed but could not be imported " + f"({type(exc).__name__}: {exc}). The torch backends are disabled.", + UserWarning, + ) + TORCH_AVAILABLE = False + MPS_AVAILABLE = False + CUDA_AVAILABLE = False try: import numba @@ -67,6 +78,13 @@ NUMBA_AVAILABLE = True except ImportError: NUMBA_AVAILABLE = False +except Exception as exc: + warnings.warn( + "numba is installed but could not be imported " + f"({type(exc).__name__}: {exc}). The numba backend is disabled.", + UserWarning, + ) + NUMBA_AVAILABLE = False # Custom kernel backends from .kernels import METAL_AVAILABLE, CUPY_AVAILABLE @@ -593,7 +611,7 @@ def _resolve_optimization( return "numba", "cpu" warnings.warn( "numba not installed, falling back to numpy. " - "Install with: poetry install --with optim_numba", + 'Install with: pip install "hypyp[numba]"', UserWarning, stacklevel=3, ) @@ -604,7 +622,7 @@ def _resolve_optimization( return cls._resolve_torch() warnings.warn( "torch not installed, falling back to numpy. " - "Install with: poetry install --with optim_torch", + 'Install with: pip install "hypyp[torch]"', UserWarning, stacklevel=3, ) diff --git a/hypyp/sync/kernels/__init__.py b/hypyp/sync/kernels/__init__.py index 7c71bed..6272463 100644 --- a/hypyp/sync/kernels/__init__.py +++ b/hypyp/sync/kernels/__init__.py @@ -6,6 +6,8 @@ (e.g., PLI, wPLI — non-linear per-timepoint operations). """ +import warnings + # Metal availability (Apple Silicon via PyObjC) try: import Metal as _Metal @@ -13,6 +15,15 @@ METAL_AVAILABLE = True except ImportError: METAL_AVAILABLE = False +except Exception as exc: + # Installed but unusable (a missing shared library, for instance): + # disable the backend instead of making `import hypyp` fail + warnings.warn( + "The Metal bindings are installed but could not be imported " + f"({type(exc).__name__}: {exc}). The Metal backend is disabled.", + UserWarning, + ) + METAL_AVAILABLE = False # CUDA availability (NVIDIA via CuPy) try: @@ -21,5 +32,12 @@ CUPY_AVAILABLE = True except ImportError: CUPY_AVAILABLE = False +except Exception as exc: + warnings.warn( + "cupy is installed but could not be imported " + f"({type(exc).__name__}: {exc}). The CUDA kernel backend is disabled.", + UserWarning, + ) + CUPY_AVAILABLE = False __all__ = ["METAL_AVAILABLE", "CUPY_AVAILABLE"] diff --git a/tests/test_sync.py b/tests/test_sync.py index 32669df..4c3d71a 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -1798,3 +1798,77 @@ class EmptyMetric(BaseMetric): n_samp = complex_signal.shape[3] with pytest.raises(NotImplementedError, match="_compute_numpy"): EmptyMetric().compute(complex_signal, n_samp, (0, 1, 3, 2)) + + +# --------------------------------------------------------------------------- +# Robustness of the optional-dependency probes and of the metric registry +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "module, flag", + [ + ("torch", "base.TORCH_AVAILABLE"), + ("numba", "base.NUMBA_AVAILABLE"), + ("cupy", "kernels.CUPY_AVAILABLE"), + ("Metal", "kernels.METAL_AVAILABLE"), + ], +) +def test_broken_optional_dependency_does_not_break_import(tmp_path, module, flag): + """An optional package that is installed but fails to import (a missing + shared library, for instance) must leave hypyp importable, with the + backend reported as unavailable and a warning that names the package.""" + import os + import subprocess + import sys + + package = tmp_path / module + package.mkdir() + (package / "__init__.py").write_text( + 'raise OSError("simulated broken install: missing shared library")\n' + ) + code = ( + "import warnings\n" + "with warnings.catch_warnings(record=True) as caught:\n" + " warnings.simplefilter('always')\n" + " import hypyp.analyses\n" + " from hypyp.sync import base, kernels\n" + f"print('FLAG', {flag})\n" + f"print('WARNED', any('{module}' in str(w.message) and 'simulated broken install' in str(w.message) for w in caught))\n" + ) + env = dict(os.environ) + env["PYTHONPATH"] = os.pathsep.join( + [str(tmp_path)] + [p for p in [env.get("PYTHONPATH")] if p] + ) + result = subprocess.run( + [sys.executable, "-c", code], capture_output=True, text=True, env=env + ) + assert result.returncode == 0, result.stderr[-2000:] + assert "FLAG False" in result.stdout + assert "WARNED True" in result.stdout + + +@pytest.mark.parametrize( + "alias, mode", + [("envelope_corr", "envcorr"), ("pow_corr", "powcorr"), ("imaginary_coh", "imcoh")], +) +def test_get_metric_accepts_the_aliases_of_compute_sync(alias, mode): + assert type(get_metric(alias)) is METRICS[mode] + assert type(get_metric(alias.upper())) is METRICS[mode] + + +def test_get_metric_unknown_mode_still_raises(): + with pytest.raises(ValueError, match="Unknown metric mode 'nope'"): + get_metric("nope") + + +@pytest.mark.parametrize("backend", ["numba", "torch"]) +def test_missing_backend_hint_names_the_pip_extra(backend): + flag = f"hypyp.sync.base.{backend.upper()}_AVAILABLE" + with patch(flag, False): + with pytest.warns(UserWarning) as record: + metric = get_metric("plv", optimization=backend) + assert metric._backend == "numpy" + messages = " ".join(str(w.message) for w in record) + assert f'pip install "hypyp[{backend}]"' in messages + assert "poetry" not in messages From 202a4af380ad946bfcae3a0fa660db933c7ef56f Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sun, 4 Oct 2026 09:25:30 -0400 Subject: [PATCH 2/4] fix(fnirs,xdf): replace four crashes with the intended behaviour Study() no longer shares its default list of dyads between objects. Recording.load_raw gives the intended random label to a Raw without subject_info or without his_id. Study.compute_wtcs_shuffle accepts with_intra. XDFImport raises ValueError for an unknown stream id and recognises a numpy integer as a stream id. Co-Authored-By: Claude Fable 5.1 --- hypyp/fnirs/recording.py | 4 ++- hypyp/fnirs/study.py | 16 +++++++--- hypyp/xdf/xdf_import.py | 10 ++++-- tests/test_fnirs.py | 67 ++++++++++++++++++++++++++++++++++++++++ tests/test_xdf.py | 14 +++++++++ 5 files changed, 103 insertions(+), 8 deletions(-) diff --git a/hypyp/fnirs/recording.py b/hypyp/fnirs/recording.py index 3686f1d..2eea5b8 100644 --- a/hypyp/fnirs/recording.py +++ b/hypyp/fnirs/recording.py @@ -172,7 +172,9 @@ def preprocess_step_choices(self): def _fill_subject_label(self): if self.subject_label == "": - self.subject_label = self.mne_raw.info["subject_info"]["his_id"] + # subject_info is None for a Raw built in memory, and his_id is optional + subject_info = self.mne_raw.info["subject_info"] or {} + self.subject_label = subject_info.get("his_id") or "" if self.subject_label == "": self.subject_label = generate_random_label(10) diff --git a/hypyp/fnirs/study.py b/hypyp/fnirs/study.py index 4610aa5..2634200 100644 --- a/hypyp/fnirs/study.py +++ b/hypyp/fnirs/study.py @@ -1,5 +1,6 @@ from typing import List, Tuple import pickle +import warnings from ..wavelet.base_wavelet import BaseWavelet from .preprocessor.base_preprocessor import BasePreprocessor @@ -18,14 +19,15 @@ class Study: dyads: List[Dyad] dyads_shuffled: List[Dyad] | None - def __init__(self, dyads: List[Dyad] = []): + def __init__(self, dyads: List[Dyad] | None = None): """ The Study object encapsulates the logic of an hyperscanning experiment. Args: - dyads (List[Dyad], optional): List of all the dyads in the study. Defaults to []. + dyads (List[Dyad], optional): List of all the dyads in the study. Defaults to a new empty list. """ - self.dyads = dyads + # a fresh list per object: a `[]` default would be shared by every Study + self.dyads = [] if dyads is None else dyads self.dyads_shuffled = None @property @@ -191,11 +193,17 @@ def get_dyads_shuffle(self) -> List[Dyad]: # TODO add as argument the number of shuffle dyads def compute_wtcs_shuffle(self, *args, **kwargs): """ - Wraps the `compute_wtcs` of all the dyads_shuffle. Arguments are directly passed to the dyads_shuffle method + Wraps the `compute_wtcs` of all the dyads_shuffle. Arguments are directly passed to the dyads_shuffle method, + except `with_intra`: shuffled dyads never compute the intra-subject pairs, so `with_intra=True` is ignored with a warning. Returns: self: the Study object itself. Useful for chaining operations """ + if kwargs.pop("with_intra", False): + warnings.warn( + "with_intra is ignored by compute_wtcs_shuffle: shuffled dyads " + "only compute the pairs between the two subjects." + ) self.dyads_shuffled = self.get_dyads_shuffle() for dyad_shuffle in self.dyads_shuffled: dyad_shuffle.compute_wtcs(*args, **kwargs, with_intra=False) diff --git a/hypyp/xdf/xdf_import.py b/hypyp/xdf/xdf_import.py index f87bf35..f5f6f81 100644 --- a/hypyp/xdf/xdf_import.py +++ b/hypyp/xdf/xdf_import.py @@ -203,9 +203,13 @@ def select_streams_by_matches(self, keyword_matches: list): for keyword_match in keyword_matches: # match stream_id found_idx = None - if type(keyword_match) == int: - stream_id = keyword_match - found_idx = self.map_id_to_idx[stream_id] + # a numpy integer is a stream id too; a bool is not + if isinstance(keyword_match, (int, np.integer)) and not isinstance( + keyword_match, bool + ): + stream_id = int(keyword_match) + # None for an unknown id, reported below like an unknown name + found_idx = self.map_id_to_idx.get(stream_id) # match stream name else: diff --git a/tests/test_fnirs.py b/tests/test_fnirs.py index 559dcbe..231f11e 100644 --- a/tests/test_fnirs.py +++ b/tests/test_fnirs.py @@ -821,3 +821,70 @@ def test_download_demos(): # def test_load_lionirs(): + + +def test_study_default_dyads_are_not_shared(): + study1 = Study() + study2 = Study() + study1.dyads.append("sentinel") + assert study2.dyads == [] + assert Study().dyads == [] + + # a list given by the caller is still used as it is + dyads = [] + assert Study(dyads).dyads is dyads + + +def _raw_array_without_subject_info(): + info = mne.create_info(["Foo 1", "Foo 2"], sfreq=10, ch_types="eeg") + return mne.io.RawArray(np.zeros((2, 100)), info) + + +def test_load_raw_without_subject_info_gets_random_label(): + raw = _raw_array_without_subject_info() + assert raw.info["subject_info"] is None + + recording = Recording().load_raw(raw, preprocess=False) + assert len(recording.subject_label) == 10 + + # a second recording gets its own label + other = Recording().load_raw(_raw_array_without_subject_info(), preprocess=False) + assert other.subject_label != recording.subject_label + + +def test_load_raw_subject_info_without_his_id_gets_random_label(): + raw = _raw_array_without_subject_info() + raw.info["subject_info"] = dict(id=1) + + recording = Recording().load_raw(raw, preprocess=False) + assert len(recording.subject_label) == 10 + + +def test_load_raw_keeps_given_label_and_his_id(): + raw = _raw_array_without_subject_info() + assert ( + Recording(subject_label="s1").load_raw(raw, preprocess=False).subject_label + == "s1" + ) + + raw.info["subject_info"] = dict(his_id="from_file") + assert Recording().load_raw(raw, preprocess=False).subject_label == "from_file" + + +def test_study_wtc_shuffle_accepts_with_intra(): + dyads = [Dyad(*get_test_recordings()) for _ in range(3)] + wtcs_kwargs = dict(ch_match=get_test_ch_match_one()) + + reference = Study(dyads).compute_wtcs_shuffle(**wtcs_kwargs) + + # the arguments of compute_wtcs can be reused as they are + study = Study(dyads).compute_wtcs_shuffle(**wtcs_kwargs, with_intra=False) + assert len(study.dyads_shuffled) == len(reference.dyads_shuffled) == 6 + + # shuffled dyads never compute the intra-subject pairs, and say so + with pytest.warns(UserWarning, match="with_intra"): + study = Study(dyads).compute_wtcs_shuffle(**wtcs_kwargs, with_intra=True) + assert len(study.dyads_shuffled) == 6 + for shuffled, expected in zip(study.dyads_shuffled, reference.dyads_shuffled): + assert len(shuffled.wtcs) == len(expected.wtcs) + assert np.all(shuffled.df["is_intra"] == False) diff --git a/tests/test_xdf.py b/tests/test_xdf.py index c16a017..30b3153 100644 --- a/tests/test_xdf.py +++ b/tests/test_xdf.py @@ -316,3 +316,17 @@ def test_montage(): ] ) xdf.set_montage("standard_1020") + + +def test_match_unknown_stream_id_raises_value_error(): + with pytest.raises(ValueError, match="No stream matching keyword '99'"): + XDFImport(file_path, select_matches=[99]) + + +def test_match_stream_by_numpy_integer_id(): + import numpy as np + + expected = XDFImport(file_path, select_matches=[STREAM_ID_EEG_1]) + for stream_id in (np.int64(STREAM_ID_EEG_1), np.int32(STREAM_ID_EEG_1)): + xdf = XDFImport(file_path, select_matches=[stream_id]) + assert xdf.selected_stream_indices == expected.selected_stream_indices From a0691f998bb02b2339bec9d447ea84c70a37f68a Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sun, 4 Oct 2026 09:25:30 -0400 Subject: [PATCH 3/4] docs(changelog): list the small fixes to sync, fnirs and xdf Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2aa7d1c..d16acac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ - `hypyp.sync`: when a backend of a `priority` list can neither run on the machine nor be computed by the metric, and no other GPU backend of the list can be used, the warning now says so (for example "'plv' has no Metal implementation") instead of "No GPU backend available" - `hypyp.sync`: a metric whose `_backend` was set to a backend it does not implement used to compute in NumPy without notice. It still computes in NumPy, now with a warning. A `_backend` that is not the name of a backend raises a `ValueError` naming the metric and the backends it implements; called through `compute_sync`, that error is still reworded as an unsupported metric (#306) - Tests: the six Metal tests that compared NumPy with NumPy are replaced by tests of the fallback itself (warning, backend and result), the dispatch of every metric to every backend it implements is checked without a GPU, and the four tests that run a Metal kernel now assert that the Metal method was called (#300) +- `import hypyp.analyses` failed when torch, numba, cupy or the Metal bindings were installed but could not be imported, for example because of a missing shared library. The backend is now reported as unavailable, with a warning that gives the original error, and everything else keeps working +- `hypyp.sync.get_metric` rejected the names `envelope_corr`, `pow_corr` and `imaginary_coh`, which `compute_sync` accepts and the documentation uses. It now accepts them; the mapping is `hypyp.sync.METRIC_ALIASES` +- `hypyp.sync`: the warnings for a missing numba or torch suggested a `poetry install` command that no longer exists; they now give `pip install "hypyp[numba]"` and `pip install "hypyp[torch]"` +- `hypyp.fnirs.Study()`: two studies created without arguments shared a single list of dyads, so a dyad appended to one appeared in the other. Each now gets its own list +- `hypyp.fnirs.Recording.load_raw` raised a `TypeError` for a `Raw` object without `subject_info`, as built in memory with `mne.io.RawArray`, and a `KeyError` when `subject_info` had no `his_id`. Both now get the random label that was intended +- `hypyp.fnirs.Study.compute_wtcs_shuffle` raised a `TypeError` when given `with_intra`, so the arguments of `compute_wtcs` could not be reused. `with_intra=False` is accepted; `with_intra=True` is ignored with a warning, since shuffled dyads only compute the pairs between the two subjects +- `XDFImport(select_matches=[...])`: an integer that is not the id of a stream raised a bare `KeyError`; it now raises the same `ValueError` as an unknown stream name. A numpy integer is recognised as a stream id ### Added - `BaseMetric.supports(backend)` tells whether a metric implements a backend, for example `PLI.supports('metal')` From 1e536c68636238a45e300cab2c86b8b6f1f748a9 Mon Sep 17 00:00:00 2001 From: Ramdam17 Date: Sun, 4 Oct 2026 10:08:10 -0400 Subject: [PATCH 4/4] fix: narrow the sync and fnirs repairs after review Study() keeps its shared default list for now: giving each study its own list changes what a second Study() computes, so it belongs to 0.7.0. compute_wtcs_shuffle accepts only the redundant with_intra=False and still refuses True. get_metric looks a name up in METRICS before trying the aliases, so a metric registered under an alias name is still returned. The probes of the optional packages tell an absent package, skipped silently, from an installed one that fails to load, which now warns whatever the exception, ImportError included. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 7 ++- hypyp/fnirs/study.py | 21 ++++---- hypyp/sync/__init__.py | 5 +- hypyp/sync/base.py | 40 ++++++++------- hypyp/sync/kernels/__init__.py | 32 ++++++------ tests/test_fnirs.py | 35 ++++++------- tests/test_sync.py | 90 ++++++++++++++++++++++++++-------- 7 files changed, 138 insertions(+), 92 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d16acac..879284e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,12 +7,11 @@ - `hypyp.sync`: when a backend of a `priority` list can neither run on the machine nor be computed by the metric, and no other GPU backend of the list can be used, the warning now says so (for example "'plv' has no Metal implementation") instead of "No GPU backend available" - `hypyp.sync`: a metric whose `_backend` was set to a backend it does not implement used to compute in NumPy without notice. It still computes in NumPy, now with a warning. A `_backend` that is not the name of a backend raises a `ValueError` naming the metric and the backends it implements; called through `compute_sync`, that error is still reworded as an unsupported metric (#306) - Tests: the six Metal tests that compared NumPy with NumPy are replaced by tests of the fallback itself (warning, backend and result), the dispatch of every metric to every backend it implements is checked without a GPU, and the four tests that run a Metal kernel now assert that the Metal method was called (#300) -- `import hypyp.analyses` failed when torch, numba, cupy or the Metal bindings were installed but could not be imported, for example because of a missing shared library. The backend is now reported as unavailable, with a warning that gives the original error, and everything else keeps working -- `hypyp.sync.get_metric` rejected the names `envelope_corr`, `pow_corr` and `imaginary_coh`, which `compute_sync` accepts and the documentation uses. It now accepts them; the mapping is `hypyp.sync.METRIC_ALIASES` +- `import hypyp.analyses` failed when the import of torch, numba, cupy or the Metal bindings raised anything other than `ImportError`, for example the `OSError` of a missing shared library. The backend is now reported as unavailable, with a warning that gives the original error. An installed package that fails with an `ImportError` used to be skipped silently and now gets the same warning; a package that is not installed is still skipped silently. A package that imports but is broken further down is not covered +- `hypyp.sync.get_metric` rejected the names `envelope_corr`, `pow_corr` and `imaginary_coh`, which `compute_sync` accepts and the documentation uses. It now accepts them; the mapping is `hypyp.sync.METRIC_ALIASES`, and a metric registered in `METRICS` under one of these names takes precedence - `hypyp.sync`: the warnings for a missing numba or torch suggested a `poetry install` command that no longer exists; they now give `pip install "hypyp[numba]"` and `pip install "hypyp[torch]"` -- `hypyp.fnirs.Study()`: two studies created without arguments shared a single list of dyads, so a dyad appended to one appeared in the other. Each now gets its own list - `hypyp.fnirs.Recording.load_raw` raised a `TypeError` for a `Raw` object without `subject_info`, as built in memory with `mne.io.RawArray`, and a `KeyError` when `subject_info` had no `his_id`. Both now get the random label that was intended -- `hypyp.fnirs.Study.compute_wtcs_shuffle` raised a `TypeError` when given `with_intra`, so the arguments of `compute_wtcs` could not be reused. `with_intra=False` is accepted; `with_intra=True` is ignored with a warning, since shuffled dyads only compute the pairs between the two subjects +- `hypyp.fnirs.Study.compute_wtcs_shuffle` raised a `TypeError` when given `with_intra=False`, so the keyword arguments of `compute_wtcs` could not be reused. That redundant argument is now accepted; `with_intra=True` is still refused - `XDFImport(select_matches=[...])`: an integer that is not the id of a stream raised a bare `KeyError`; it now raises the same `ValueError` as an unknown stream name. A numpy integer is recognised as a stream id ### Added diff --git a/hypyp/fnirs/study.py b/hypyp/fnirs/study.py index 2634200..ae3f29f 100644 --- a/hypyp/fnirs/study.py +++ b/hypyp/fnirs/study.py @@ -1,6 +1,5 @@ from typing import List, Tuple import pickle -import warnings from ..wavelet.base_wavelet import BaseWavelet from .preprocessor.base_preprocessor import BasePreprocessor @@ -19,15 +18,14 @@ class Study: dyads: List[Dyad] dyads_shuffled: List[Dyad] | None - def __init__(self, dyads: List[Dyad] | None = None): + def __init__(self, dyads: List[Dyad] = []): """ The Study object encapsulates the logic of an hyperscanning experiment. Args: - dyads (List[Dyad], optional): List of all the dyads in the study. Defaults to a new empty list. + dyads (List[Dyad], optional): List of all the dyads in the study. Defaults to []. """ - # a fresh list per object: a `[]` default would be shared by every Study - self.dyads = [] if dyads is None else dyads + self.dyads = dyads self.dyads_shuffled = None @property @@ -194,16 +192,17 @@ def get_dyads_shuffle(self) -> List[Dyad]: def compute_wtcs_shuffle(self, *args, **kwargs): """ Wraps the `compute_wtcs` of all the dyads_shuffle. Arguments are directly passed to the dyads_shuffle method, - except `with_intra`: shuffled dyads never compute the intra-subject pairs, so `with_intra=True` is ignored with a warning. + which is always called with `with_intra=False`. Passing `with_intra=False` here is accepted and changes nothing. Returns: self: the Study object itself. Useful for chaining operations """ - if kwargs.pop("with_intra", False): - warnings.warn( - "with_intra is ignored by compute_wtcs_shuffle: shuffled dyads " - "only compute the pairs between the two subjects." - ) + # with_intra=False is passed below; drop a redundant one from the caller, + # so that the keyword arguments of compute_wtcs can be reused as they are + if kwargs.get("with_intra") is False: + kwargs = { + key: value for key, value in kwargs.items() if key != "with_intra" + } self.dyads_shuffled = self.get_dyads_shuffle() for dyad_shuffle in self.dyads_shuffled: dyad_shuffle.compute_wtcs(*args, **kwargs, with_intra=False) diff --git a/hypyp/sync/__init__.py b/hypyp/sync/__init__.py index 87a533b..dd27bd8 100644 --- a/hypyp/sync/__init__.py +++ b/hypyp/sync/__init__.py @@ -143,7 +143,10 @@ def get_metric( ... priority=['numba', 'metal']) # custom priority """ mode_lower = mode.lower() - mode_lower = METRIC_ALIASES.get(mode_lower, mode_lower) + # a name registered in METRICS wins, so an alias never hides a metric + # that a user registered under that name + if mode_lower not in METRICS: + mode_lower = METRIC_ALIASES.get(mode_lower, mode_lower) if mode_lower not in METRICS: available = ", ".join(METRICS.keys()) raise ValueError(f"Unknown metric mode '{mode}'. Available: {available}") diff --git a/hypyp/sync/base.py b/hypyp/sync/base.py index ef82b57..516624e 100644 --- a/hypyp/sync/base.py +++ b/hypyp/sync/base.py @@ -50,41 +50,45 @@ # Check optional dependency availability +def _is_absent(exc: BaseException, package: str) -> bool: + """True if `exc` only says that the optional `package` is not installed. + + Any other failure (a missing shared library, an incompatible version, an + error while probing a device) means the package is installed but + unusable, which deserves a warning instead of a silent fallback. + """ + return isinstance(exc, ModuleNotFoundError) and exc.name == package + + try: import torch TORCH_AVAILABLE = True MPS_AVAILABLE = torch.backends.mps.is_available() CUDA_AVAILABLE = torch.cuda.is_available() -except ImportError: - TORCH_AVAILABLE = False - MPS_AVAILABLE = False - CUDA_AVAILABLE = False except Exception as exc: - # Installed but unusable (a missing shared library, for instance): - # disable the backend instead of making `import hypyp` fail - warnings.warn( - "torch is installed but could not be imported " - f"({type(exc).__name__}: {exc}). The torch backends are disabled.", - UserWarning, - ) TORCH_AVAILABLE = False MPS_AVAILABLE = False CUDA_AVAILABLE = False + if not _is_absent(exc, "torch"): + warnings.warn( + "torch is installed but could not be loaded " + f"({type(exc).__name__}: {exc}). The torch backends are disabled.", + UserWarning, + ) try: import numba NUMBA_AVAILABLE = True -except ImportError: - NUMBA_AVAILABLE = False except Exception as exc: - warnings.warn( - "numba is installed but could not be imported " - f"({type(exc).__name__}: {exc}). The numba backend is disabled.", - UserWarning, - ) NUMBA_AVAILABLE = False + if not _is_absent(exc, "numba"): + warnings.warn( + "numba is installed but could not be loaded " + f"({type(exc).__name__}: {exc}). The numba backend is disabled.", + UserWarning, + ) # Custom kernel backends from .kernels import METAL_AVAILABLE, CUPY_AVAILABLE diff --git a/hypyp/sync/kernels/__init__.py b/hypyp/sync/kernels/__init__.py index 6272463..9399bcf 100644 --- a/hypyp/sync/kernels/__init__.py +++ b/hypyp/sync/kernels/__init__.py @@ -8,36 +8,36 @@ import warnings +# A package that is not installed is skipped silently; one that is installed +# but fails to load (a missing shared library, for instance) disables the +# backend with a warning instead of making `import hypyp` fail. + # Metal availability (Apple Silicon via PyObjC) try: import Metal as _Metal METAL_AVAILABLE = True -except ImportError: - METAL_AVAILABLE = False except Exception as exc: - # Installed but unusable (a missing shared library, for instance): - # disable the backend instead of making `import hypyp` fail - warnings.warn( - "The Metal bindings are installed but could not be imported " - f"({type(exc).__name__}: {exc}). The Metal backend is disabled.", - UserWarning, - ) METAL_AVAILABLE = False + if not (isinstance(exc, ModuleNotFoundError) and exc.name == "Metal"): + warnings.warn( + "The Metal bindings are installed but could not be loaded " + f"({type(exc).__name__}: {exc}). The Metal backend is disabled.", + UserWarning, + ) # CUDA availability (NVIDIA via CuPy) try: import cupy as _cp CUPY_AVAILABLE = True -except ImportError: - CUPY_AVAILABLE = False except Exception as exc: - warnings.warn( - "cupy is installed but could not be imported " - f"({type(exc).__name__}: {exc}). The CUDA kernel backend is disabled.", - UserWarning, - ) CUPY_AVAILABLE = False + if not (isinstance(exc, ModuleNotFoundError) and exc.name == "cupy"): + warnings.warn( + "cupy is installed but could not be loaded " + f"({type(exc).__name__}: {exc}). The CUDA kernel backend is disabled.", + UserWarning, + ) __all__ = ["METAL_AVAILABLE", "CUPY_AVAILABLE"] diff --git a/tests/test_fnirs.py b/tests/test_fnirs.py index 231f11e..40363a2 100644 --- a/tests/test_fnirs.py +++ b/tests/test_fnirs.py @@ -823,18 +823,6 @@ def test_download_demos(): # def test_load_lionirs(): -def test_study_default_dyads_are_not_shared(): - study1 = Study() - study2 = Study() - study1.dyads.append("sentinel") - assert study2.dyads == [] - assert Study().dyads == [] - - # a list given by the caller is still used as it is - dyads = [] - assert Study(dyads).dyads is dyads - - def _raw_array_without_subject_info(): info = mne.create_info(["Foo 1", "Foo 2"], sfreq=10, ch_types="eeg") return mne.io.RawArray(np.zeros((2, 100)), info) @@ -871,20 +859,25 @@ def test_load_raw_keeps_given_label_and_his_id(): assert Recording().load_raw(raw, preprocess=False).subject_label == "from_file" -def test_study_wtc_shuffle_accepts_with_intra(): +def test_study_wtc_shuffle_accepts_redundant_with_intra(): + from pandas.testing import assert_frame_equal + dyads = [Dyad(*get_test_recordings()) for _ in range(3)] wtcs_kwargs = dict(ch_match=get_test_ch_match_one()) reference = Study(dyads).compute_wtcs_shuffle(**wtcs_kwargs) + assert len(reference.dyads_shuffled) == 6 - # the arguments of compute_wtcs can be reused as they are + # the keyword arguments of compute_wtcs can be reused as they are study = Study(dyads).compute_wtcs_shuffle(**wtcs_kwargs, with_intra=False) - assert len(study.dyads_shuffled) == len(reference.dyads_shuffled) == 6 - - # shuffled dyads never compute the intra-subject pairs, and say so - with pytest.warns(UserWarning, match="with_intra"): - study = Study(dyads).compute_wtcs_shuffle(**wtcs_kwargs, with_intra=True) assert len(study.dyads_shuffled) == 6 for shuffled, expected in zip(study.dyads_shuffled, reference.dyads_shuffled): - assert len(shuffled.wtcs) == len(expected.wtcs) - assert np.all(shuffled.df["is_intra"] == False) + assert len(expected.wtcs) == 1 + assert len(expected.df) > 0 + assert shuffled.label == expected.label + assert_frame_equal(shuffled.df, expected.df) + np.testing.assert_array_equal(shuffled.wtcs[0].W, expected.wtcs[0].W) + + # with_intra=True is still refused, as before + with pytest.raises(TypeError, match="with_intra"): + Study(dyads).compute_wtcs_shuffle(**wtcs_kwargs, with_intra=True) diff --git a/tests/test_sync.py b/tests/test_sync.py index 4c3d71a..e64960b 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -1805,36 +1805,26 @@ class EmptyMetric(BaseMetric): # --------------------------------------------------------------------------- -@pytest.mark.parametrize( - "module, flag", - [ - ("torch", "base.TORCH_AVAILABLE"), - ("numba", "base.NUMBA_AVAILABLE"), - ("cupy", "kernels.CUPY_AVAILABLE"), - ("Metal", "kernels.METAL_AVAILABLE"), - ], -) -def test_broken_optional_dependency_does_not_break_import(tmp_path, module, flag): - """An optional package that is installed but fails to import (a missing - shared library, for instance) must leave hypyp importable, with the - backend reported as unavailable and a warning that names the package.""" +def _import_hypyp_with_fake_package(tmp_path, module, flag, init_source): + """Import hypyp in a fresh interpreter where `module` resolves to a fake + package whose `__init__` is `init_source`. Returns the availability flag + and the messages of the warnings that name the module.""" + import json import os import subprocess import sys package = tmp_path / module package.mkdir() - (package / "__init__.py").write_text( - 'raise OSError("simulated broken install: missing shared library")\n' - ) + (package / "__init__.py").write_text(init_source) code = ( - "import warnings\n" + "import json, warnings\n" "with warnings.catch_warnings(record=True) as caught:\n" " warnings.simplefilter('always')\n" " import hypyp.analyses\n" " from hypyp.sync import base, kernels\n" - f"print('FLAG', {flag})\n" - f"print('WARNED', any('{module}' in str(w.message) and 'simulated broken install' in str(w.message) for w in caught))\n" + f"messages = [str(w.message) for w in caught if '{module}' in str(w.message)]\n" + f"print('RESULT', json.dumps([bool({flag}), messages]))\n" ) env = dict(os.environ) env["PYTHONPATH"] = os.pathsep.join( @@ -1844,8 +1834,54 @@ def test_broken_optional_dependency_does_not_break_import(tmp_path, module, flag [sys.executable, "-c", code], capture_output=True, text=True, env=env ) assert result.returncode == 0, result.stderr[-2000:] - assert "FLAG False" in result.stdout - assert "WARNED True" in result.stdout + line = [l for l in result.stdout.splitlines() if l.startswith("RESULT ")][-1] + return json.loads(line[len("RESULT ") :]) + + +OPTIONAL_PACKAGES = [ + ("torch", "base.TORCH_AVAILABLE"), + ("numba", "base.NUMBA_AVAILABLE"), + ("cupy", "kernels.CUPY_AVAILABLE"), + ("Metal", "kernels.METAL_AVAILABLE"), +] + + +@pytest.mark.parametrize("module, flag", OPTIONAL_PACKAGES) +@pytest.mark.parametrize( + "error", + [ + 'OSError("simulated broken install: missing shared library")', + 'ImportError("simulated broken install: cannot load a symbol")', + 'ModuleNotFoundError("simulated broken install", name="a_dependency")', + 'RuntimeError("simulated broken install: version mismatch")', + ], +) +def test_broken_optional_dependency_does_not_break_import( + tmp_path, module, flag, error +): + """An optional package that is installed but fails to load must leave + hypyp importable, with the backend reported as unavailable and a warning + that names the package and gives the original error.""" + available, messages = _import_hypyp_with_fake_package( + tmp_path, module, flag, f"raise {error}\n" + ) + assert available is False + assert len(messages) == 1 + assert "simulated broken install" in messages[0] + + +@pytest.mark.parametrize("module, flag", OPTIONAL_PACKAGES) +def test_absent_optional_dependency_stays_silent(tmp_path, module, flag): + """A package that is simply not installed disables its backend without + any warning, as before.""" + available, messages = _import_hypyp_with_fake_package( + tmp_path, + module, + flag, + f'raise ModuleNotFoundError("No module named {module!r}", name="{module}")\n', + ) + assert available is False + assert messages == [] @pytest.mark.parametrize( @@ -1857,6 +1893,18 @@ def test_get_metric_accepts_the_aliases_of_compute_sync(alias, mode): assert type(get_metric(alias.upper())) is METRICS[mode] +def test_get_metric_registered_name_wins_over_alias(): + """A metric that a user registered under the name of an alias is still + the one returned.""" + + class Custom(METRICS["plv"]): + pass + + with patch.dict(METRICS, {"pow_corr": Custom}): + assert type(get_metric("pow_corr")) is Custom + assert type(get_metric("pow_corr")) is METRICS["powcorr"] + + def test_get_metric_unknown_mode_still_raises(): with pytest.raises(ValueError, match="Unknown metric mode 'nope'"): get_metric("nope")