Skip to content

Commit dc62a27

Browse files
committed
Fix KeyError on partial/empty rescoring config
A user-provided rescoring dict (e.g. "rescoring": {} or {"model": "lda"}) is passed through as-is by the config cascade -- same as feature_generators and psm_generator, where an override is taken at face value rather than merged with defaults (CascadeConfig's max_recursion_depth stops it from reaching into these nested dicts either way). rescoring.py and parse_psms.py were indexing config["rescoring"]["train_fdr"]/["model"] directly, which KeyErrors as soon as either key is missing. Fixed by using dict.get(..., default) where a value is needed before calling ristretto, and by passing config["rescoring"] to ristretto.rescore() via **kwargs instead of naming train_fdr/model explicitly, so ristretto's own defaults (train_fdr=0.01, model="svm") apply to whatever the user's config omitted.
1 parent 49104a3 commit dc62a27

3 files changed

Lines changed: 42 additions & 5 deletions

File tree

ms2rescore/parse_psms.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ def parse_psms(config: Dict, psm_list: Union[PSMList, None]) -> PSMList:
4040
# Remove invalid AAs and find decoys first, so score direction can be inferred from them
4141
psm_list = _remove_invalid_aa(psm_list)
4242
_find_decoys(psm_list, config["id_decoy_pattern"])
43-
train_fdr = config["rescoring"]["train_fdr"] if config["rescoring"] else 0.01
43+
train_fdr = config["rescoring"].get("train_fdr", 0.01) if config["rescoring"] else 0.01
4444
lower_score_is_better = infer_score_direction(psm_list, train_fdr)
4545

4646
# Filter by PSM rank

ms2rescore/rescoring.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ def evaluate_before(psm_list: PSMList, config: Dict) -> RescoreResult:
147147
original_mask = np.array([_is_original_psm(psm) for psm in psm_list])
148148
psm_list = psm_list[original_mask]
149149

150-
train_fdr = config["rescoring"]["train_fdr"] if config["rescoring"] else 0.01
150+
train_fdr = config["rescoring"].get("train_fdr", 0.01) if config["rescoring"] else 0.01
151151
features_df = build_features_dataframe(psm_list, set(), infer_score_direction(psm_list, train_fdr))
152152
return _trim_and_evaluate(
153153
features_df,
@@ -197,7 +197,9 @@ def rescore(psm_list: PSMList, config: Dict, output_file_root: str) -> Tuple[PSM
197197
198198
"""
199199
feature_names = {f for psm in psm_list for f in psm.rescoring_features}
200-
lower_score_is_better = infer_score_direction(psm_list, config["rescoring"]["train_fdr"])
200+
lower_score_is_better = infer_score_direction(
201+
psm_list, config["rescoring"].get("train_fdr", 0.01)
202+
)
201203
features_df = build_features_dataframe(psm_list, feature_names, lower_score_is_better)
202204

203205
peptide_col = "peptide"
@@ -212,10 +214,11 @@ def rescore(psm_list: PSMList, config: Dict, output_file_root: str) -> Tuple[PSM
212214
protein_col=protein_col,
213215
feature_cols=sorted(feature_names),
214216
decoy_pattern=decoy_pattern,
215-
model=config["rescoring"]["model"],
216-
train_fdr=config["rescoring"]["train_fdr"],
217217
n_jobs=int(config["processes"]),
218218
multi_rank_rescoring=True,
219+
# train_fdr/model; ristretto's own kwarg defaults apply to any key a partial
220+
# user-provided rescoring dict omitted.
221+
**config["rescoring"],
219222
)
220223
final_result = _trim_and_evaluate(
221224
ml_result.psms,

tests/test_rescoring.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
"""Tests for ms2rescore.rescoring: the ristretto integration layer."""
22

3+
from unittest.mock import patch
4+
35
import numpy as np
46
import pandas as pd
57
from psm_utils import PSM, PSMList
@@ -193,6 +195,38 @@ def test_rescore_respects_configured_model():
193195
assert ((q >= 0) & (q <= 1)).all()
194196

195197

198+
def test_rescore_handles_partial_rescoring_config():
199+
"""A partial rescoring dict (e.g. only `model` or only `train_fdr`, as produced by the
200+
config cascade for a user override of just one key) must not KeyError on the other."""
201+
psm_list = _make_psm_list(n_spectra=30, seed=19)
202+
203+
for rescoring_config in ({"model": "lda"}, {"train_fdr": 0.1}):
204+
config = {**BASE_CONFIG, "rescoring": rescoring_config}
205+
new_psm_list, after_result = rescoring.rescore(psm_list, config, "unused-output-root")
206+
assert len(new_psm_list) == len(after_result.psms)
207+
208+
209+
def test_rescore_empty_rescoring_config_falls_back_to_ristretto_defaults():
210+
"""An empty rescoring dict (the config cascade's result for `"rescoring": {}`) must not
211+
KeyError -- train_fdr/model should simply be omitted from the ristretto.rescore() call, so
212+
ristretto's own defaults (train_fdr=0.01, model="svm") apply. Whether ristretto's fit
213+
itself converges on this tiny fixture at the (stricter) default train_fdr is irrelevant
214+
here -- only that the right kwargs reach it.
215+
"""
216+
psm_list = _make_psm_list(n_spectra=30, seed=19)
217+
config = {**BASE_CONFIG, "rescoring": {}}
218+
219+
with patch.object(rescoring.ristretto, "rescore", wraps=rescoring.ristretto.rescore) as m:
220+
try:
221+
rescoring.rescore(psm_list, config, "unused-output-root")
222+
except rescoring.RescoringError:
223+
pass
224+
225+
passed_kwargs = m.call_args.kwargs
226+
assert "train_fdr" not in passed_kwargs
227+
assert "model" not in passed_kwargs
228+
229+
196230
def test_rescore_multi_rank_output_keeps_multiple_ranks_per_spectrum():
197231
psm_list = _make_psm_list(n_spectra=30, ranks_per_spectrum=2, seed=6)
198232
config = {**BASE_CONFIG, "max_psm_rank_output": 2}

0 commit comments

Comments
 (0)