-
Notifications
You must be signed in to change notification settings - Fork 4
fix(protspace): derive missing sequence length from fasta #401
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.
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
Open
FlorinSenoner
wants to merge
6
commits into
main
Choose a base branch
from
feat/336-fasta-sequence-length
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
b02521f
fix(protspace): derive missing length from fasta
FlorinSenoner 9d409fe
Merge remote-tracking branch 'origin/main' into feat/336-fasta-sequen…
FlorinSenoner 0e000cb
fix(annotations): enrich complete cache lengths from fasta
FlorinSenoner 5cab319
fix(annotate): pass fasta sequences to annotation manager
FlorinSenoner b584582
test(annotate): hoist UniProtRetriever import to module level
tsenoner 5826d8d
fix(annotations): handle fasta length edge cases
FlorinSenoner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import pandas as pd | ||
| from typer.testing import CliRunner | ||
|
|
||
| from protspace.cli.app import app | ||
| from protspace.data.annotations.retrievers.uniprot_retriever import ( | ||
| ProteinAnnotations, | ||
| UniProtRetriever, | ||
| ) | ||
|
|
||
|
|
||
| def test_annotate_fasta_derives_missing_length_from_normalized_sequence( | ||
| tmp_path, monkeypatch | ||
| ): | ||
| """The FASTA-backed CLI path must supply sequences to the annotation manager.""" | ||
| fasta = tmp_path / "input.fasta" | ||
| fasta.write_text(">custom|custom_protein|description\nMPEPTIDE\n") | ||
| output = tmp_path / "annotations.parquet" | ||
|
|
||
| monkeypatch.setattr( | ||
| UniProtRetriever, | ||
| "fetch_annotations", | ||
| lambda self: [ | ||
| ProteinAnnotations( | ||
| identifier="custom_protein", | ||
| annotations={"length": ""}, | ||
| ) | ||
| ], | ||
| ) | ||
|
|
||
| result = CliRunner().invoke( | ||
| app, | ||
| [ | ||
| "annotate", | ||
| "-i", | ||
| str(fasta), | ||
| "-a", | ||
| "length", | ||
| "-o", | ||
| str(output), | ||
| ], | ||
| ) | ||
|
|
||
| assert result.exit_code == 0, result.output | ||
| assert pd.read_parquet(output).loc[0, "length"] == "8" |
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Apply the fallback before the complete-cache early return
This helper only runs once
ProteinAnnotationManager.to_pd()is reached, butReductionPipeline._fetch_annotations()returnscached_dfdirectly whenall_annotations.parquetalready contains the requested columns (andprotspace preparekeeps that cache by default). I reproduced this at this head with cachedcustom_protein,length=''plus FASTAMPEPTIDE: the pipeline loggedUsing cached annotationsand returned'', not'8'. Users rerunning an existing output after upgrading therefore continue to see N/A. Route the complete-cache branch through the same enrichment (or invalidate/migrate cached empty lengths) and add a warm-cache pipeline regression test.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 0e000cb. I independently reproduced the complete-cache branch returning an empty length for cached
custom_proteinplus FASTAMPEPTIDE(cached_length='', expected8). The cache-hit branch now enriches a copy of the selected DataFrame through the same missing-only scalar resolver used byProteinAnnotationManager; non-empty cached lengths remain authoritative andall_annotations.parquetis not rewritten. The new warm-cache pipeline regression failed as['', '110'] != ['8', '110']before the fix and passes afterward. Fresh evidence: focused affected tests 160 passed; full Python suite 753 passed, 2 skipped; Ruff clean/143 formatted; strict OpenSpec valid;pnpm precommitpassed; bundle contract 11 passed.