Legacy `WaveformExtractor` load path calls `set_params` before `_handle_backward_compatibility_on_load`, rejecting deprecated metric names that the compat handler is designed to migrate

[galenlynch]

_read_old_waveforms_extractor_binary in spikeinterface/core/waveforms_extractor_backwards_compatibility.py invokes ext.set_params() (which triggers _set_params() validation) before running ext._handle_backward_compatibility_on_load(). For extensions whose on-disk params.json contains metric names that were renamed in 0.104 (e.g. peak_to_valley, velocity_above, l_ratio, …), validation raises a ValueError before the compat handler ever runs — even though the compat handler exists specifically to migrate those names.

The result is that legacy WaveformExtractor folders that SI is otherwise still able to read cannot be loaded on SI ≥ 0.104 without rewriting params.json on disk, which is not always possible (e.g. Code Ocean data assets are immutable).

Reproduction

Create or obtain a WaveformExtractor folder that has a template_metrics or quality_metrics extension whose params.json contains any deprecated metric name. For example, a template_metrics/params.json with:

{
  "metric_names": ["peak_to_valley", "velocity_above", "velocity_below"],
  "metric_params": {
    "velocity_above": {"min_channels_for_velocity": 3, "min_r2_velocity": 0.5}
  }
}

Then:

import spikeinterface as si
from spikeinterface.core import load_sorting_analyzer_or_waveforms
analyzer = load_sorting_analyzer_or_waveforms("/path/to/legacy_waveforms")

yields:

ValueError: The metric 'peak_to_valley' has been re-named or re-organized.
You can now compute it using the metric name 'peak_to_trough_duration'.
The metric 'velocity_above' has been re-named or re-organized.
You can now compute it using the metric name 'velocity_fits'.
...

Root cause

waveforms_extractor_backwards_compatibility.py:631-637 runs:

new_params = ext._set_params()
updated_params = make_ext_params_up_to_date(ext, params, new_params)
ext.set_params(**updated_params, save=False)        # ← validation fires here
if ext.need_backward_compatibility_on_load:
    ext._handle_backward_compatibility_on_load()    # ← never reached

The deprecated-name validation in BaseMetricExtension._set_params (analyzer_extension_core.py:1127-1134) exists to catch code paths that are constructing extensions with old names at runtime. But it's also triggered on the legacy load path, where the deprecated names come from on-disk params.json — which is exactly the case the per-extension _handle_backward_compatibility_on_load handlers are designed to handle.

ComputeTemplateMetrics._handle_backward_compatibility_on_load (metrics/template/template_metrics.py:100-171) has full, authoritative migration logic for num_positive_peaks/num_negative_peaks → number_of_peaks, velocity_above/velocity_below → velocity_fits (including renaming min_channels_for_velocity → min_channels etc. inside metric_params), peak_to_valley → peak_to_trough_duration, and peak_trough_ratio → waveform_ratios. ComputeQualityMetrics._handle_backward_compatibility_on_load does the same for isolation_distance/l_ratio → mahalanobis. None of it runs, because validation aborts the load first.

Proposed fix

Catch the deprecation ValueError in _read_old_waveforms_extractor_binary, install the raw params on the extension so the compat handler can see them, run the handler, and replay set_params with the migrated values:

# waveforms_extractor_backwards_compatibility.py, replacing lines 634-637
try:
    ext.set_params(**updated_params, save=False)
except ValueError as e:
    if "has been re-named or re-organized" not in str(e):
        raise
    # Legacy params.json contains deprecated metric names. Install the
    # raw params so _handle_backward_compatibility_on_load can see them,
    # run the compat handler to rename metric_names AND migrate
    # metric_params sub-keys, then replay set_params with migrated values.
    ext.params = dict(updated_params)
    if ext.need_backward_compatibility_on_load:
        ext._handle_backward_compatibility_on_load()
    ext.set_params(**ext.params, save=False)
else:
    if ext.need_backward_compatibility_on_load:
        ext._handle_backward_compatibility_on_load()

The contained change: one function, no signature changes, non-legacy folders take the existing fast path. Happy to send a PR if this approach looks reasonable.

Environment

• spikeinterface: 0.104.1
• python: 3.13

[chrishalcrow]

Hi @galenlynch , thanks for this!

I'm curious if there's an even simpler fix. The actual error arises here:

spikeinterface/src/spikeinterface/core/analyzer_extension_core.py

Line 1126 in 89381fe

else:

The intention of this error message was for when a user tries to pass a deprecated metric name e.g.

compute_quality_metrics(analyzer, metric_names = ["old_metric_name"])

But it's getting in the way here on load - not our intention. Maybe we could add a flag to skip this check, and skip it for this case? The _handle_backward_compatibility_on_load should then handle backwards compatibility on load.

@galenlynch Could you check if simply removing the error raising linked above fixes this?

[galenlynch]

That seems like a very reasonable approach! We'll try it out.

[galenlynch]

@chrishalcrow thanks for the suggestion. I think the core problem was trying to validate before migrating. Both my original suggestion and yours are workarounds.

We can reorder _read_old_waveforms_extractor_binary so the compat handler runs before validation, rather than adding a flag to skip or soften the check. The deprecated-name validation in _set_params is correct for the programmatic path (analyzer.compute("quality_metrics", metric_names=["old_name"])). It's only the legacy load path where it shouldn't fire.

Here's the current order (lines 632-637):

new_params = ext._set_params()                                        # 1. get defaults
updated_params = make_ext_params_up_to_date(ext, params, new_params)  # 2. merge on-disk with defaults
ext.set_params(**updated_params, save=False)                          # 3. validate + set — (errors)
if ext.need_backward_compatibility_on_load:
    ext._handle_backward_compatibility_on_load()                      # 4. migrate — never reached

Two things go wrong before the compat handler gets a chance to run:

1. Step 3: _set_params rejects deprecated names like peak_to_valley or l_ratio.
2. Step 2: make_ext_params_up_to_date strips any metric_params keys that don't exist in the new defaults — so deprecated sub-params (e.g. velocity_above.min_channels_for_velocity) are
   silently dropped before the compat handler could migrate them to their new names (e.g. velocity_fits.min_channels).

Running the compat handler on the raw on-disk params first is what SortingAnalyzer already does for non-legacy folders, so the change would just make WaveformExtractor have the same logical flow:

# Install raw on-disk params and run compat handler first,
# matching what AnalyzerExtension.load does for non-legacy folders.
ext.params = dict(params)
if ext.need_backward_compatibility_on_load:
    ext._handle_backward_compatibility_on_load()                         # original 4

# Now merge and validate — deprecated names are already migrated.
new_params = ext._set_params()                                           # original 1
updated_params = make_ext_params_up_to_date(ext, ext.params, new_params) # original 2
ext.set_params(**updated_params, save=False)                             # original 3

After the compat handler migrates the names, set_params sees only valid current names and passes cleanly. No need for a skip flag or try-catch.

I'll send a PR.

[chrishalcrow]

Nice! Thanks @galenlynch , I'll review once you get the PR done