fix(results): support pyOpenMS 3.5.0 IdXMLFile.load signature - #32
Merged
Conversation
Loading any idXML on the cluster failed with:
Exception: can not handle type of ('..._comet.idXML', [], [])
pyOpenMS 3.5.0 changed the third parameter of IdXMLFile.load()/store()
from a libcpp_vector[PeptideIdentification], which accepted an ordinary
Python list, to a dedicated PeptideIdentificationList container. Passing
a list there matches no overload, so autowrap's dispatcher raises before
any file I/O happens. protein_ids is unaffected and still takes a list.
Bisected across locally installed releases: 3.1.0, 3.2.0, 3.3.0, 3.4.0
and 3.4.1 all accept a list; 3.5.0 does not. requirements.txt pins
pyopenms==3.5.0 and Dockerfile builds OpenMS release/3.5.0, so deployed
images hit this on every results page, while dev environments still on
3.3.x do not.
Add load_idxml() in results_helpers, which feature-detects
PeptideIdentificationList and normalises the result back to a plain list,
and route the three call sites through it. Feature detection rather than
a version check keeps the app working on 3.4.x and earlier too.
IdXMLFile is no longer referenced in WorkflowTest, so drop the import.
Verified end-to-end against a generated idXML on 3.1.0, 3.3.0, 3.4.0
(legacy list path) and 3.5.0 (new container), all yielding identical
parsed output. The added regression tests reproduce the exact production
error message when the fix is reverted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
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.
Problem
Every workflow run on the cluster dies as soon as it tries to read a Comet result:
Root cause
pyOpenMS 3.5.0 changed the signature of
IdXMLFile.load()/store(). The thirdparameter went from a
libcpp_vector[PeptideIdentification], which accepted an ordinaryPython
list, to a dedicatedPeptideIdentificationListcontainer:Passing
[]matches no overload, so autowrap's dispatcher raises before any file I/O —which is why the message names the file but never touches it.
protein_idsis unchangedand still takes a plain list.
Bisected across locally installed releases:
IdXMLFile().load(path, [], [])can not handle type ofrequirements.txtpinspyopenms==3.5.0andDockerfilebuilds OpenMSrelease/3.5.0,so deployed images always hit this, while dev environments still on 3.3.x never do.
That's the version mismatch — the deployment is ahead of the code, not behind it.
Other pyOpenMS loaders in the repo (
MzMLFile,ConsensusXMLFile,ParamXMLFile) passproperly wrapped objects rather than plain lists, and were verified unaffected on 3.5.0.
Fix
Add
load_idxml()toresults_helpers, which feature-detects the new container andnormalises the result back to a plain
listso every caller is unchanged:Feature detection rather than a version check keeps the app working on 3.4.x and earlier,
so local dev environments don't have to be upgraded in lockstep. The three call sites
(
idxml_to_df,parse_idxml, and the inline copy inWorkflowTest) now route through it;IdXMLFileis no longer referenced inWorkflowTest, so that import is dropped.Note
PeptideIdentificationListexposespush_back, notappend— worth remembering forany future code that builds one to hand to
store().Verification
3.5.0 (new container) — all four produce identical parsed output through
load_idxml,idxml_to_dfandparse_idxml.tests/test_results_helpers.py. Reverting the fix makesthem fail with the exact production error message,
Exception: can not handle type of ('...', [], []).requirements.txt, i.e. pyopenms3.5.0): 102 passed.
E0611by dropping the now-unused import.🤖 Generated with Claude Code
https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku