From 400fb8fd2ae8c405bdca8ee4269bc30b8580929b Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Fri, 23 Oct 2026 03:49:47 +0330 Subject: [PATCH 1/4] Fix legacy Mahalanobis metric parameters on load --- ...est_waveforms_extractor_backwards_compatibility.py | 3 +++ src/spikeinterface/metrics/quality/quality_metrics.py | 11 ++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/spikeinterface/core/tests/test_waveforms_extractor_backwards_compatibility.py b/src/spikeinterface/core/tests/test_waveforms_extractor_backwards_compatibility.py index e75f3be156..3e476c0ddb 100644 --- a/src/spikeinterface/core/tests/test_waveforms_extractor_backwards_compatibility.py +++ b/src/spikeinterface/core/tests/test_waveforms_extractor_backwards_compatibility.py @@ -238,6 +238,9 @@ def test_load_legacy_we_with_deprecated_metrics(create_cache_folder, tmp_path): # qm_params should have been renamed to metric_params assert "qm_params" not in qm.params assert "metric_params" in qm.params + assert qm.params["metric_params"]["mahalanobis"] == {} + assert "isolation_distance" not in qm.params["metric_params"] + assert "l_ratio" not in qm.params["metric_params"] # template_metrics: deprecated names should be migrated tm = sorting_analyzer.get_extension("template_metrics") diff --git a/src/spikeinterface/metrics/quality/quality_metrics.py b/src/spikeinterface/metrics/quality/quality_metrics.py index 0e50a4eb79..cb86d6e987 100644 --- a/src/spikeinterface/metrics/quality/quality_metrics.py +++ b/src/spikeinterface/metrics/quality/quality_metrics.py @@ -61,7 +61,16 @@ def _handle_backward_compatibility_on_load(self): self.params["metric_params"] = qm_params del self.params["qm_params"] - # handle metric names change: isolation_distance/l_ratio merged into mahalanobis + # handle metric names and parameters change: isolation_distance/l_ratio merged into mahalanobis + legacy_mahalanobis_params = [] + for legacy_name in ("isolation_distance", "l_ratio"): + if legacy_name in self.params["metric_params"]: + legacy_mahalanobis_params.append(self.params["metric_params"].pop(legacy_name)) + if legacy_mahalanobis_params and "mahalanobis" not in self.params["metric_params"]: + if any(params != legacy_mahalanobis_params[0] for params in legacy_mahalanobis_params[1:]): + raise ValueError("Legacy isolation_distance and l_ratio parameters do not match") + self.params["metric_params"]["mahalanobis"] = legacy_mahalanobis_params[0] + if "isolation_distance" in self.params["metric_names"]: self.params["metric_names"].remove("isolation_distance") if "mahalanobis" not in self.params["metric_names"]: From 30ad4c30e10970e136cbb3741253c5040e38286d Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Fri, 23 Oct 2026 10:27:38 +0330 Subject: [PATCH 2/4] Allow decoding trusted legacy fixtures in serialization CI --- .github/workflows/cross_version_serialization.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/cross_version_serialization.yml b/.github/workflows/cross_version_serialization.yml index 5bf4dcf4f4..ec1f3aa020 100644 --- a/.github/workflows/cross_version_serialization.yml +++ b/.github/workflows/cross_version_serialization.yml @@ -112,6 +112,10 @@ jobs: python .github/scripts/serialization/serialize_objects.py "$SI_SERIALIZATION_FIXTURES_DIR" - name: Load fixtures with the branch + env: + # Legacy Zarr fixtures contain pickled objects. These fixtures are trusted: + # they were generated locally by the preceding step in this job. + NUMCODECS_ALLOW_UNSAFE_PICKLE_DECODE: "1" run: | # [full] here too: loading the analyzer extensions unpickles e.g. the # scikit-learn PCA models written at generation time From 6eca58ab5dd4d6ddc6aea49259776ef06d89befd Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Fri, 23 Oct 2026 10:57:43 +0330 Subject: [PATCH 3/4] Scope legacy pickle decoding to generated fixture tests --- .../serialization/test_cross_version_compatibility.py | 6 +++++- .github/workflows/cross_version_serialization.yml | 4 ---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/scripts/serialization/test_cross_version_compatibility.py b/.github/scripts/serialization/test_cross_version_compatibility.py index bdb7d019ba..85ef140c4e 100644 --- a/.github/scripts/serialization/test_cross_version_compatibility.py +++ b/.github/scripts/serialization/test_cross_version_compatibility.py @@ -13,6 +13,7 @@ from pathlib import Path import pytest +import numcodecs.pickles from spikeinterface.core import load @@ -24,9 +25,12 @@ @pytest.mark.parametrize("entry,fmt", _CASES, ids=[f"{entry['id']}-{fmt}" for entry, fmt in _CASES]) -def test_load_old_serialized_object(entry, fmt): +def test_load_old_serialized_object(entry, fmt, monkeypatch): fixture = FIXTURES_DIR / f"{entry['id']}{FIXTURE_SUFFIX[fmt]}" assert fixture.exists(), f"missing fixture {fixture}" + # Legacy Zarr fixtures contain pickled objects generated locally by this CI job. + # Limit the opt-in to this test and support older Numcodecs without this flag. + monkeypatch.setattr(numcodecs.pickles, "ALLOW_UNSAFE_PICKLE_DECODE", True, raising=False) obj = load(fixture) check_extra_data = None if (FIXTURES_DIR / f"{entry['id']}.json").exists(): diff --git a/.github/workflows/cross_version_serialization.yml b/.github/workflows/cross_version_serialization.yml index ec1f3aa020..5bf4dcf4f4 100644 --- a/.github/workflows/cross_version_serialization.yml +++ b/.github/workflows/cross_version_serialization.yml @@ -112,10 +112,6 @@ jobs: python .github/scripts/serialization/serialize_objects.py "$SI_SERIALIZATION_FIXTURES_DIR" - name: Load fixtures with the branch - env: - # Legacy Zarr fixtures contain pickled objects. These fixtures are trusted: - # they were generated locally by the preceding step in this job. - NUMCODECS_ALLOW_UNSAFE_PICKLE_DECODE: "1" run: | # [full] here too: loading the analyzer extensions unpickles e.g. the # scikit-learn PCA models written at generation time From 6564c25e5affcc6120d5596cbf8816a749d0cc1d Mon Sep 17 00:00:00 2001 From: AtomicGlance Date: Sat, 24 Oct 2026 11:31:22 +0330 Subject: [PATCH 4/4] Warn on conflicting legacy metric parameters and narrow PR scope --- .../serialization/test_cross_version_compatibility.py | 6 +----- ...est_waveforms_extractor_backwards_compatibility.py | 11 +++++++++++ src/spikeinterface/metrics/quality/quality_metrics.py | 6 +++++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/.github/scripts/serialization/test_cross_version_compatibility.py b/.github/scripts/serialization/test_cross_version_compatibility.py index 85ef140c4e..bdb7d019ba 100644 --- a/.github/scripts/serialization/test_cross_version_compatibility.py +++ b/.github/scripts/serialization/test_cross_version_compatibility.py @@ -13,7 +13,6 @@ from pathlib import Path import pytest -import numcodecs.pickles from spikeinterface.core import load @@ -25,12 +24,9 @@ @pytest.mark.parametrize("entry,fmt", _CASES, ids=[f"{entry['id']}-{fmt}" for entry, fmt in _CASES]) -def test_load_old_serialized_object(entry, fmt, monkeypatch): +def test_load_old_serialized_object(entry, fmt): fixture = FIXTURES_DIR / f"{entry['id']}{FIXTURE_SUFFIX[fmt]}" assert fixture.exists(), f"missing fixture {fixture}" - # Legacy Zarr fixtures contain pickled objects generated locally by this CI job. - # Limit the opt-in to this test and support older Numcodecs without this flag. - monkeypatch.setattr(numcodecs.pickles, "ALLOW_UNSAFE_PICKLE_DECODE", True, raising=False) obj = load(fixture) check_extra_data = None if (FIXTURES_DIR / f"{entry['id']}.json").exists(): diff --git a/src/spikeinterface/core/tests/test_waveforms_extractor_backwards_compatibility.py b/src/spikeinterface/core/tests/test_waveforms_extractor_backwards_compatibility.py index 3e476c0ddb..cb4dc5f8fe 100644 --- a/src/spikeinterface/core/tests/test_waveforms_extractor_backwards_compatibility.py +++ b/src/spikeinterface/core/tests/test_waveforms_extractor_backwards_compatibility.py @@ -252,6 +252,17 @@ def test_load_legacy_we_with_deprecated_metrics(create_cache_folder, tmp_path): assert "metrics_kwargs" not in tm.params assert "metric_params" in tm.params + # Conflicting legacy parameters must not prevent loading the analyzer. + params_path = we_folder / "quality_metrics" / "params.json" + with open(params_path) as f: + legacy_params = json.load(f) + legacy_params["qm_params"]["l_ratio"] = {"legacy_parameter": 1} + with open(params_path, "w") as f: + json.dump(legacy_params, f) + with pytest.warns(UserWarning, match="Using isolation_distance parameters for mahalanobis"): + reloaded = load_waveforms_backwards(we_folder, sorting=sorting, output="SortingAnalyzer") + assert reloaded.get_extension("quality_metrics").params["metric_params"]["mahalanobis"] == {} + # @pytest.mark.skip("This test is run locally") # def test_read_old_waveforms_extractor_zarr(): diff --git a/src/spikeinterface/metrics/quality/quality_metrics.py b/src/spikeinterface/metrics/quality/quality_metrics.py index cb86d6e987..222ba1932d 100644 --- a/src/spikeinterface/metrics/quality/quality_metrics.py +++ b/src/spikeinterface/metrics/quality/quality_metrics.py @@ -68,7 +68,11 @@ def _handle_backward_compatibility_on_load(self): legacy_mahalanobis_params.append(self.params["metric_params"].pop(legacy_name)) if legacy_mahalanobis_params and "mahalanobis" not in self.params["metric_params"]: if any(params != legacy_mahalanobis_params[0] for params in legacy_mahalanobis_params[1:]): - raise ValueError("Legacy isolation_distance and l_ratio parameters do not match") + warnings.warn( + "Legacy isolation_distance and l_ratio parameters do not match. " + "Using isolation_distance parameters for mahalanobis and discarding l_ratio parameters.", + stacklevel=2, + ) self.params["metric_params"]["mahalanobis"] = legacy_mahalanobis_params[0] if "isolation_distance" in self.params["metric_names"]: