Rename Outcome -> EffectCandidate; document purpose; drop description field - #381
Merged
Conversation
…tion Three cleanups on the multi-outcome wrapper: 1. Rename ``Outcome`` -> ``EffectCandidate``. The class wraps an Effect with provenance — "candidate effect with metadata" reads honestly. Module ``varcode.outcomes`` -> ``varcode.effect_candidates``; helper ``outcomes_from_candidates`` -> ``candidates_from_effects``; test file renamed in lockstep. 2. Drop ``EffectCandidate.description``. Pure passthrough to ``effect.short_description``; producers wanting a richer narrative attach it to the underlying Effect, not the wrapper. ``make_rna_outcome(description=...)`` kwarg also removed. 3. Module docstring now explains *why* the wrapper exists rather than just describing it: the same MutationEffect instance can appear in multiple multi-outcome contexts with different metadata per context (e.g. a splice candidate re-surfaced by the SV annotator with a different source tag). Without the wrapper, the alternatives are copy-everywhere, metadata-on-Effect (wrong: forces one labeling across contexts), or sidecar id() dicts. Also: scrub aspirational "isovar" / "exacto" / "longread_assembly" example tags from varcode docstrings — those integrations don't exist yet; naming them as canonical examples implied a dependency that hadn't shipped. Hard rename, no deprecation alias. The class was only exported as varcode.Outcome — update imports to varcode.EffectCandidate. The MultiOutcomeEffect.candidates / .outcomes dual access stays (distinct, complementary; raw vs wrapped); the "back-compat" framing on .candidates is dropped. 1188 tests pass.
18 tasks
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.
Re-creating PR #380, which auto-closed when its base branch (#379's
generic-phasing-api) was deleted on merge. Rebased onto current main.Summary
Three cleanups on the multi-outcome wrapper:
Rename
Outcome→EffectCandidate. The class wraps anEffectwith provenance — "candidate effect with metadata" reads honestly. Modulevarcode.outcomes→varcode.effect_candidates; helperoutcomes_from_candidates→candidates_from_effects; test file renamed in lockstep.Drop
EffectCandidate.description. Pure passthrough toeffect.short_description; producers wanting a richer narrative attach it to the underlying Effect, not the wrapper.make_rna_outcome(description=...)kwarg also removed.Module docstring explains why the wrapper exists. Previously framed as "minimal type for unifying multi-outcome shapes" with the actual rationale buried. Now leads with: the same
MutationEffectinstance can appear in multiple multi-outcome contexts with different metadata per context (e.g. a splice candidate re-surfaced by the SV annotator with a differentsourcetag andsv_typein evidence). Without the wrapper, the alternatives are copy-everywhere, metadata-on-Effect (wrong: forces one labeling across contexts), or sidecarid()dicts. Documents that the wrapper is the cheapest answer.Also: scrub aspirational
"isovar"/"exacto"/"longread_assembly"example tags from varcode docstrings — those integrations don't exist yet; naming them as canonical examples implied a dependency that hadn't shipped.What didn't change
MultiOutcomeEffect.candidatesand.outcomesare kept as two complementary first-class accessors (rawtuple[MutationEffect, ...]vs wrappedtuple[EffectCandidate, ...]). The misleading "back-compat for 2.x" framing on.candidatesis dropped — both serve distinct, legitimate needs.I tried collapsing to a single accessor (
.candidatesreturning wrapped,.effectsfor raw) and reverted. The migration broke ~30 splice/RNA tests becauseSpliceOutcomeSet.candidateshas deep history of returningSpliceCandidate(a different domain-specific type) and serialization round-trips depend on the shape. Collapsing requires either widespread test migration or back-compat shims that defeat the cleanup. Kept the pragmatic landing — documented sharply.Test plan
pytest tests/— 1188 pass, 2 pre-existing skips.pytest.raises(TypeError)on the removeddescription=kwarg so it can't slip back via copy/paste.tests/test_outcomes.py→tests/test_effect_candidates.py(git rename + test method names updatedtest_outcome_*→test_effect_candidate_*).Outcomeoroutcomes_from_candidatesreferences anywhere."isovar"/"exacto"/"longread_assembly"strings in varcode source.References
Outcomeintroduction (whose "harmonized interface" framing this PR replaces).