Fix SMG exclusion in get_components: check whole component b4 scoring PHROGs - #67
Open
gbouras13 wants to merge 1 commit into
Open
Fix SMG exclusion in get_components: check whole component b4 scoring PHROGs#67gbouras13 wants to merge 1 commit into
gbouras13 wants to merge 1 commit into
Conversation
… scoring PHROGs component_utils.get_components had two related gaps in how it kept bacterial single-copy marker genes (SMGs) out of candidate phage components, despite the docstring saying "components with PHROGs and no SMGs": 1. Multi-unitig components: the loop checked each unitig for an SMG and broke on the first one found, but a `break` only stops the loop -- it doesn't clear PHROG-category flags a unitig earlier in the same component already set. So whether a mixed bacterial/phage component got excluded depended on which unitig igraph happened to iterate first, not on whether an SMG was actually present. A component with a PHROG-bearing unitig visited before an SMG-bearing one survived with the marker gene intact; the same two unitigs in the opposite order got correctly excluded. 2. Single-unitig components had no SMG check at all -- a lone unitig carrying both a marker gene and a PHROG hit was always kept. Fixed by checking every unitig in a component for an SMG membership up front, in its own pass, before any PHROG scoring happens for either the multi- or single-unitig case. A component with any SMG is skipped outright; there's no point scoring PHROG evidence for one that's excluded either way. Verified with a standalone test (four cases: PHROG-then-SMG order, SMG-then- PHROG order, a single unitig with both, and a clean SMG-free control) -- before the fix, the first case was incorrectly kept; after, all mixed cases are excluded and the clean case is unaffected. Also ran the real pipeline against phables' own bundled test_data/ fixture: component count is unchanged there (6), as expected -- that fixture has no marker-gene hits at all, so it doesn't exercise this path either way, but confirms nothing else regressed.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
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.
component_utils.get_components had two related gaps in how it kept bacterial single-copy marker genes (SMGs) out of candidate phage components, despite the docstring saying "components with PHROGs and no SMGs":
Multi-unitig components: the loop checked each unitig for an SMG and broke on the first one found, but a
breakonly stops the loop -- it doesn't clear PHROG-category flags a unitig earlier in the same component already set. So whether a mixed bacterial/phage component got excluded depended on which unitig igraph happened to iterate first, not on whether an SMG was actually present. A component with a PHROG-bearing unitig visited before an SMG-bearing one survived with the marker gene intact; the same two unitigs in the opposite order got correctly excluded.Single-unitig components had no SMG check at all -- a lone unitig carrying both a marker gene and a PHROG hit was always kept.
Fixed by checking every unitig in a component for an SMG membership up front, in its own pass, before any PHROG scoring happens for either the multi- or single-unitig case. A component with any SMG is skipped outright; there's no point scoring PHROG evidence for one that's excluded either way.
Verified with a standalone test (four cases: PHROG-then-SMG order, SMG-then- PHROG order, a single unitig with both, and a clean SMG-free control) -- before the fix, the first case was incorrectly kept; after, all mixed cases are excluded and the clean case is unaffected. Also ran the real pipeline against phables' own bundled test_data/ fixture: component count is unchanged there (6), as expected -- that fixture has no marker-gene hits at all, so it doesn't exercise this path either way, but confirms nothing else regressed.