Skip to content

Unify multi-outcome machinery: drop SpliceCandidate, drop .outcomes accessor, EffectCandidate everywhere #382

Description

@iskandr

Background

Follow-up from #381 (OutcomeEffectCandidate rename). That PR aligned naming but stopped short of the deeper architectural cleanup. Recap of the current mess and the target.

The mess today

MultiOutcomeEffect subclasses each have their own incompatible storage and accessor patterns:

Subclass Internal storage Public .candidates returns Public .outcomes returns
MultiOutcomeEffect (base) n/a — subclasses provide tuple[MutationEffect, ...] (raw) tuple[EffectCandidate, ...] (wrapped via _with_extra_outcomes)
SpliceOutcomeSet self._splice_candidates (tuple[SpliceCandidate]) tuple[SpliceCandidate, ...] (a different type entirely) tuple[EffectCandidate, ...] (wraps SpliceCandidate fields into EffectCandidate.evidence)
StructuralVariantEffect self._candidates + self._cryptic_candidates + self._splice_candidates raw tuple[MutationEffect, ...] from _candidates wrapped, unions the three lists
PhaseCandidateSet self._candidates_raw + self._hypotheses sorted raw tuple wraps with phase metadata
ExonicSpliceSite inline (self, alternate_effect) raw 2-tuple default wrap

The class EffectCandidate is what's returned from .outcomes. Not from .candidates. That's the inversion that reads as messy: the type named EffectCandidate is not in the accessor named .candidates.

SpliceCandidate is a separate domain-specific dataclass that overlaps with EffectCandidate's purpose: both wrap an effect with metadata. SpliceCandidate's fields (outcome, plausibility, coding_effect, description, predicted_class_name, mutant_transcript) all fit cleanly into EffectCandidate's shape:

  • coding_effect (or placeholder) → EffectCandidate.effect
  • plausibilityEffectCandidate.probability
  • outcomeEffectCandidate.evidence["splice_outcome"]
  • mutant_transcripteffect.mutant_transcript (the inner Effect already has this slot)
  • predicted_class_name → recoverable from effect.__class__.__name__
  • description → optional evidence["description"], or dropped

So SpliceCandidate exists because of historical evolution, not architectural necessity.

Target

class EffectCandidate:                           # unchanged
    effect: MutationEffect                       # the wrapped effect (real or placeholder)
    source: str = \"varcode\"
    probability: float | None
    evidence: Mapping

class MultiOutcomeEffect:
    @property
    def candidates(self) -> tuple[EffectCandidate, ...]: ...
    @property
    def effects(self) -> tuple[MutationEffect, ...]:
        return tuple(c.effect for c in self.candidates)

# Drop:
#   MultiOutcomeEffect.outcomes  (everywhere)
#   MultiOutcomeEffect._with_extra_outcomes  (rename to _with_extra_candidates)
#   varcode.splice_outcomes.SpliceCandidate  (entire class)
#   SpliceOutcomeSet._outcome_effect  (placeholder construction moves to build time)

Uniformity goal: every MultiOutcomeEffect subclass exposes the same shape (tuple[EffectCandidate, ...]). No special-case types, no special-case accessors.

Scope

Production code (varcode/)

  • Drop MultiOutcomeEffect.outcomes property and _with_extra_outcomes helper. Rename _extra_outcomes slot → _extra_candidates.
  • Add MultiOutcomeEffect.effects convenience property.
  • SpliceOutcomeSet: rewrite to store tuple[EffectCandidate, ...] directly. The _build_*_candidate helpers in splice_outcomes.py (~10 sites) return EffectCandidate instead of SpliceCandidate. Placeholder Effect (PredictedIntronRetention, PredictedCrypticSpliceSite, ExonLoss, Intronic) constructed at build time, not access time. mutant_transcript attached to the inner effect instance.
  • Delete SpliceCandidate class entirely. Drop its export from varcode/__init__.py.
  • StructuralVariantEffect: move the per-context wrapping logic from outcomes property into the candidates property. Subclass internal storage stays private.
  • PhaseCandidateSet: same migration. most_likely returns inner Effect.
  • ExonicSpliceSite: candidates wraps (self, alternate_effect) as EffectCandidate entries.
  • HaplotypeEffect: trivial 1-candidate set; same migration.
  • apply_rna_evidence_to_effects: write to _extra_candidates. observed_outcomes Protocol return type stays Sequence[EffectCandidate] (the wrapper is what's appended).
  • apply_phase_resolver_to_effects: should already be fine; verify.

Tests (~30 files)

  • Mass-rename effect.outcomes reads → effect.candidates.
  • Splice tests: migrate c.plausibilityc.probability, c.outcomec.evidence[\"splice_outcome\"], c.coding_effectc.effect, c.predicted_class_namec.effect.__class__.__name__ (or drop).
  • Delete SpliceCandidate(...) direct-construction tests (test_splice_candidate_is_frozen, test_splice_candidate_equality, etc.) — the class is gone.
  • test_splice_outcome_set_json_round_trip etc.: serialization shape changes. Hard break documented.
  • apply_rna_evidence_to_effects tests: _extra_outcomes_extra_candidates.

Docs

  • docs/germline.md: replace .outcomes references with .candidates. Document that EffectCandidate is the uniform shape across multi-outcome effects.
  • docs/api.md: drop SpliceCandidate entry; keep EffectCandidate. Document .candidates and .effects as the canonical accessors on MultiOutcomeEffect.
  • Module docstrings: scrub outcomes references.

CHANGELOG entry

Hard break, no deprecation alias. SpliceCandidate users need to read c.effect.X / c.probability / c.evidence[\"splice_outcome\"] instead of c.coding_effect / c.plausibility / c.outcome.

Acceptance

  • grep -rn '\.outcomes' varcode/ tests/ returns only references to the splice-outcome enum / module / splice_outcomes=True kwarg / observed_outcomes Protocol method. No effect.outcomes reads.
  • grep -rn 'SpliceCandidate' varcode/ tests/ returns zero.
  • 1188+ tests pass.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions