Skip to content

Fix failing tests: decimate in chunks and skops #4792

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chrishalcrow merged 3 commits into from
Sep 17, 2026
+16 −19
Merged

Fix failing tests: decimate in chunks and skops #4792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4852282
fix: relax tolerance in decimate chunks tests
alejoe91 Sep 17, 2026
818fc6b
fix: untrusted models skops
alejoe91 Sep 16, 2026
c05ce7d
fix: skops trusted issue in examples
alejoe91 Sep 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions examples/tutorials/curation/plot_1_automated_curation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
# model and some metadata about the model.

model, model_info = sc.load_model(
repo_id = "SpikeInterface/toy_tetrode_model",
trusted = ['numpy.dtype']
repo_id="SpikeInterface/toy_tetrode_model",
trust_model=True
)


Expand Down Expand Up @@ -87,9 +87,9 @@
# a confidence for each unit contained in the ``sorting_analyzer``.

labels = sc.model_based_label_units(
sorting_analyzer = sorting_analyzer,
repo_id = "SpikeInterface/toy_tetrode_model",
trusted = ['numpy.dtype']
sorting_analyzer=sorting_analyzer,
repo_id="SpikeInterface/toy_tetrode_model",
trust_model=True
)

print(labels)
Expand Down
10 changes: 2 additions & 8 deletions src/spikeinterface/curation/model_based_curation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,14 +473,8 @@ def _load_model_from_folder(model_folder=None, model_name=None, trust_model=Fals
skops_file = skops_files[0]

if trust_model and trusted is None:
try:
model = skio.load(skops_file)
except UntrustedTypesFoundException as e:
exception_msg = str(e)
# the exception message contains the list of untrusted objects. The following
# search assumes it is the only list in the message.
string_list = re.search(r"\[(.*?)\]", exception_msg).group()
trusted = [list_item for list_item in string_list.split("'") if len(list_item) > 2]
untrusted = skio.get_untrusted_types(file=skops_file)
trusted = untrusted
Comment on lines +476 to +477

@chrishalcrow chrishalcrow Sep 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the scikit-learn warning message:

- sklearn.tree._tree.Tree: sklearn.tree._tree.Tree (the shared node storage for DecisionTree*, RandomForest*, ExtraTrees*, and GradientBoosting* models) stores raw node indices (left_child, right_child, feature) that scikit-learn indexes into without bounds checking. A malicious file can set these to out-of-range values: skops loads the object successfully, but calling .predict() on it can then crash the process (segfault) or read out-of-bounds memory. If you created the file yourself or otherwise fully trust its source, you can load it with trusted=["sklearn.tree._tree.Tree"].
    Only add the specific types you have reviewed and trust to the `trusted` argument; avoid passing everything reported by get_untrusted_types() just to make a file load.

I think it's probably ok, because we force the user to set trust_model = True so it's their responsibility. But it's a bit awkward.

@alejoe91 alejoe91 Sep 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's the same strategy we had before, I dodn't know there was a function for it so I was parsing the exception...

@chrishalcrow chrishalcrow Sep 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of the exception was to force users to think for a second before blindly trusting.


model = skio.load(skops_file, trusted=trusted)
_patch_sklearn_imputer_compatibility(model)
Expand Down
13 changes: 8 additions & 5 deletions src/spikeinterface/curation/tests/test_model_based_curation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ def model(trained_pipeline_path):
It has been trained locally and, when applied to `sorting_analyzer_for_unitrefine_curation` will label its 10 units with
the following labels: [1,1,1,1,1,1,1,1,1,1,0,0,0,0,0,0,0,0,0,0]."""

model = load_model(trained_pipeline_path, trusted=["numpy.dtype"])
model = load_model(
trained_pipeline_path,
trust_model=True,
)
return model


Expand Down Expand Up @@ -66,7 +69,7 @@ def test_metric_ordering_independence(sorting_analyzer_for_unitrefine_curation,
prediction_prob_dataframe_1 = model_based_label_units(
sorting_analyzer=sorting_analyzer_for_unitrefine_curation,
model_folder=trained_pipeline_path,
trusted=["numpy.dtype"],
trust_model=True,
)

sorting_analyzer_for_unitrefine_curation.compute(
Expand All @@ -76,7 +79,7 @@ def test_metric_ordering_independence(sorting_analyzer_for_unitrefine_curation,
prediction_prob_dataframe_2 = model_based_label_units(
sorting_analyzer=sorting_analyzer_for_unitrefine_curation,
model_folder=trained_pipeline_path,
trusted=["numpy.dtype"],
trust_model=True,
)

assert prediction_prob_dataframe_1.equals(prediction_prob_dataframe_2)
Expand Down Expand Up @@ -194,7 +197,7 @@ def test_exception_raised_when_metric_params_not_equal(sorting_analyzer_for_unit
"template_metrics", metric_names=["half_width", "peak_to_trough_duration", "number_of_peaks"]
)

model, model_info = load_model(model_folder=trained_pipeline_path, trusted=["numpy.dtype"])
model, model_info = load_model(model_folder=trained_pipeline_path, trust_model=True)
model_based_classification = ModelBasedClassification(sorting_analyzer_for_unitrefine_curation, model)

# an error should be raised if `enforce_metric_params` is True
Expand All @@ -215,6 +218,6 @@ def test_exception_raised_when_metric_params_not_equal(sorting_analyzer_for_unit
"template_metrics", metric_names=["half_width", "peak_to_trough_duration"]
)

model, model_info = load_model(model_folder=trained_pipeline_path, trusted=["numpy.dtype"])
model, model_info = load_model(model_folder=trained_pipeline_path, trust_model=True)
model_based_classification = ModelBasedClassification(sorting_analyzer_for_unitrefine_curation, model)
model_based_classification._check_params_for_classification(enforce_metric_params=True, model_info=model_info)
2 changes: 1 addition & 1 deletion src/spikeinterface/preprocessing/tests/test_decimate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def test_decimate_antialias_by_chunks(decimation_factor):
for chunk_size in [137, int(decimated_rate * 2)]:
rec3 = rec2.save(format="memory", chunk_size=chunk_size, n_jobs=1, progress_bar=False)
traces3 = rec3.get_traces()
np.testing.assert_allclose(traces3, traces2, rtol=1e-6, atol=1e-6)
np.testing.assert_allclose(traces3, traces2, rtol=1e-5, atol=1e-5)


@pytest.mark.parametrize("decimation_factor", [6, 10])
Expand Down
Loading