Consolidate predictor wrappers + BigMHC fixes + docstring cleanup - #248
Merged
Conversation
… cleanup Follow-up to the code-review correctness PR (#247). Structural + cosmetic: net -129 lines in the wrapper files with no behavior change (except the BigMHC fixes below). Consolidation (new mhctools/wrapper_base.py): - NewModelPredictorMixin holds the members that were copy-pasted across the standalone wrappers: predict_dataframe, supported_kinds, __repr__, and peptide normalization (single-string + strip/upper). - AlleleFreePredictor adds the allele-independent kind_support shape. - Calis / DeepTAP / ERAMER now subclass AlleleFreePredictor; BigMHC uses NewModelPredictorMixin. Each keeps only its constructor, _default_pred_kind, tool-specific validation (bounds/messages differ), and scoring. BigMHC consistency fixes (were flagged in review): - Build predictions from the *normalized* peptide, not the raw input, so "siinfekl " is no longer scored as SIINFEKL but returned verbatim. - Rename _pred_kind -> _default_pred_kind to match every other wrapper (generic consumers can now call it uniformly). Minor code: - eramer: early-return on empty input (was loading the whole PWM first); drop the unreachable score-is-None branch; delete the dead _predictor_name. - calis/deeptap: delete the dead _predictor_name. - pred: add reduce_op(kind, field) and use it in PeptideResult.best_by and AnnotationSpec.direction_op (the max/min mapping was written twice). - mixmhc2pred: resolve the %Rank_/Score_ column positions once, not per row. Docs / LLMism cleanup: - Trim the copy-pasted "Note on interpretation" caveats to one sentence each (calis/prime/eramer/deeptap); drop PRIME's benchmark editorializing and the marketing/hedging phrasing; remove **bold** from reST docstrings. - Fix stale docstrings: calis position_weights ("leading slice"), eramer's class docstring (add $ERAMER_PWM to the resolution order). - Trim the bloated best_direction docstring, filler class docstrings, an over-defensive annotate comment, and the verbose nettcr suppression docstring. Tests: new tests/test_wrapper_base.py covers the shared base and asserts the wrappers inherit it (and BigMHC standardizes on _default_pred_kind). Full public subset: 496 passed. Version 3.28.0 -> 3.29.0. Claude-Session: https://claude.ai/code/session_01LZahFhBSCiehXTESCYQ7wG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second of two PRs from the code review (follows #247). This is the structural + cosmetic half: net −129 lines in the wrapper files, no behavior change except the BigMHC fixes.
Consolidation — new
mhctools/wrapper_base.pyThe four standalone wrappers each re-implemented the same boilerplate. Extracted into:
NewModelPredictorMixin—predict_dataframe,supported_kinds,__repr__, and peptide normalization (single-string + strip/upper).AlleleFreePredictor— the allele-independentkind_supportshape.Calis/DeepTAP/ERAMERnow subclassAlleleFreePredictor;BigMHCusesNewModelPredictorMixin. Each keeps only its constructor,_default_pred_kind, tool-specific validation (bounds/messages genuinely differ), and scoring.BigMHC consistency fixes (flagged in review)
"siinfekl "was scored asSIINFEKLbut returned verbatim. Now builds from the normalized peptide._pred_kind→_default_pred_kind: aligns BigMHC with every other wrapper so a generic consumer can call it uniformly.(The ProcessingPredictor family keeps its own internally-consistent
_pred_kind; BigMHC's_find_bigmhc_dirbad-path case is already caught by the__init__src/bigmhc.pycheck, and BigMHC stays intentionally pan-length so no length validation was added.)Minor code
score is Nonebranch._predictor_namein calis/deeptap/eramer (they hardcode the literal).reduce_op(kind, field), used bybest_byandAnnotationSpec.direction_op(the max/min mapping was written twice).%Rank_/Score_column positions once, not per row.Docs / LLMism cleanup
**bold**from reST docstrings.calis.position_weights("leading slice"),ERAMERclass docstring (add$ERAMER_PWM).best_directiondocstring, filler class docstrings, an over-defensiveannotatecomment, and the verbose nettcr suppression docstring.Tests
New
tests/test_wrapper_base.pycovers the shared base and asserts the wrappers inherit it (and that BigMHC standardizes on_default_pred_kind). Full public subset: 496 passed. The MixMHC2pred and DeepTAP/ERAMER e2e were re-run locally against the real tools.Version
3.28.0 → 3.29.0.https://claude.ai/code/session_01LZahFhBSCiehXTESCYQ7wG