You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Varcode's phasing API has Isovar in its name everywhere — IsovarPhaseResolver, IsovarAssemblyProvider, IsovarResultAdapter, etc. — even though varcode doesn't import isovar and doesn't need to know it exists.
This is wrong on two counts:
It implies a dependency that shouldn't exist. Varcode → isovar would be a circular dep (since isovar already declares varcode>=4.15.0). The Isovar-prefixed identifiers in varcode's public API make readers reasonably suspect that loop is real. It isn't, but the naming creates the smell.
It pretends one upstream is special. The Protocol shape — "tell me whether two variants are observed on the same supporting reads" — is generic. Long-read tools (HiFi, ONT), assembly-based pipelines (hifiasm, Verkko, Exacto), other RNA-phasers, and hand-rolled test stubs can all satisfy it without being Isovar. Naming the Protocol IsovarAssemblyProvider forces those callers to either lie about their identity or build a wrapper. (See: openvax/isovar's existing VarcodeAdapter and the new IsovarReadPhasing from Add IsovarReadPhasing adapter for RNA-read variant phasing isovar#183 — both are Isovar implementing a varcode-defined contract; varcode should let them do that without naming the contract after Isovar.)
Principle
Varcode provides a generic Protocol. Implementations live wherever they make sense — typically in the package whose data is being adapted. Varcode does not import or name those implementations.
This keeps the dependency strictly one-way (isovar → varcode), matches what's already true at the package level, and stops varcode from pretending it has special knowledge of any particular upstream.
Proposed renames (all in varcode/phasing.py and varcode/__init__.py)
Today
New
IsovarAssemblyProvider (Protocol)
Removed. Split into ReadPhasingSource (Protocol) + MutantTranscriptSource (Protocol).
has_contig(variant, transcript)
has_evidence(variant) on ReadPhasingSource
variants_in_contig(variant, transcript)
partners_in_cis(variant) on ReadPhasingSource
mutant_transcript(variant, transcript)
Unchanged signature, lives on MutantTranscriptSource
IsovarPhaseResolver
ReadPhaseResolver
ReadPhaseResolver requires a ReadPhasingSource. The mutant-transcript channel is a separate optional concern — when the wrapped provider also satisfies MutantTranscriptSource, the resolver routes mutant_transcript(...) calls to it; otherwise that method returns None.
This is the same protocol split that issue #377 already proposed; this issue subsumes #377 by making the rename fully generic instead of half-renamed. Close #377 in favor of this one.
What happens on the isovar side
Isovar keeps its adapters. They're legitimate — isovar depends on varcode, so it can ship classes that satisfy varcode-defined Protocols:
isovar.VarcodeAdapter (the heavy one) implements both ReadPhasingSourceandMutantTranscriptSource. Needs minor surgery to match the new method names; also probably worth renaming (drop the "Varcode" prefix) since with this change there's no varcode-named Protocol to adapt to — it's just "adapter from IsovarResult to the generic interfaces." Suggested: IsovarPhasingAndTranscriptProvider or just two classes (IsovarReadPhasing + IsovarMutantTranscript).
That's a follow-up isovar PR, blocking on this issue landing.
End-to-end demo (acceptance test)
fromvarcodeimportload_vcf, GermlineContext, ReadPhaseResolverfromisovarimportrun_isovar, IsovarReadPhasingtumor=load_vcf(\"tumor.vcf\", genome=\"GRCh38\")
germline=GermlineContext.from_germline_vcf(\"normal.vcf\")
isovar_results=run_isovar(variants=tumor, alignment_file=\"tumor.rna.bam\")
phaser=ReadPhaseResolver(IsovarReadPhasing(isovar_results))
effects=tumor.effects(germline=germline, phase_resolver=phaser)
# PhaseCandidateSet outputs collapse wherever Isovar saw somatic +# germline on the same RNA reads. No Isovar-named symbol in varcode.
The varcode side has zero mention of "Isovar" anywhere — including in the imports. Same demo works against any other ReadPhasingSource implementation (long-read tools, hand-rolled stubs, future RNA-phasing packages) without varcode noticing.
Scope
Define ReadPhasingSource + MutantTranscriptSource Protocols in varcode/phasing.py.
Remove IsovarAssemblyProvider.
Rename IsovarPhaseResolver → ReadPhaseResolver. Update its constructor to accept any ReadPhasingSource; expose mutant_transcript(...) only when the wrapped source also satisfies MutantTranscriptSource.
Update tests/test_isovar_phase_resolver.py — rename to tests/test_read_phase_resolver.py; rename StubAssemblyProvider → StubReadPhasingSource with the new methods.
Update varcode/__init__.py re-exports.
Update docs/germline.md — replace IsovarPhaseResolver references with ReadPhaseResolver. Keep one sentence noting Isovar as the most common implementer.
CHANGELOG entry under Breaking.
Coordinate companion isovar PR: rename VarcodeAdapter methods to match new protocols. Consider splitting VarcodeAdapter into two classes (one per protocol).
Out of scope
The MutantTranscript construction logic in isovar's VarcodeAdapter stays in isovar. Varcode doesn't need to know how Isovar builds it — only that something implements MutantTranscriptSource.mutant_transcript(variant, transcript) -> MutantTranscript | None.
Naming for downstream RNA-evidence resolvers (IsovarRNAEvidenceResolver etc.) — separate issue if/when those exist.
Problem
Varcode's phasing API has Isovar in its name everywhere —
IsovarPhaseResolver,IsovarAssemblyProvider,IsovarResultAdapter, etc. — even though varcode doesn't import isovar and doesn't need to know it exists.This is wrong on two counts:
It implies a dependency that shouldn't exist. Varcode → isovar would be a circular dep (since isovar already declares
varcode>=4.15.0). The Isovar-prefixed identifiers in varcode's public API make readers reasonably suspect that loop is real. It isn't, but the naming creates the smell.It pretends one upstream is special. The Protocol shape — "tell me whether two variants are observed on the same supporting reads" — is generic. Long-read tools (HiFi, ONT), assembly-based pipelines (hifiasm, Verkko, Exacto), other RNA-phasers, and hand-rolled test stubs can all satisfy it without being Isovar. Naming the Protocol
IsovarAssemblyProviderforces those callers to either lie about their identity or build a wrapper. (See: openvax/isovar's existingVarcodeAdapterand the newIsovarReadPhasingfrom Add IsovarReadPhasing adapter for RNA-read variant phasing isovar#183 — both are Isovar implementing a varcode-defined contract; varcode should let them do that without naming the contract after Isovar.)Principle
This keeps the dependency strictly one-way (isovar → varcode), matches what's already true at the package level, and stops varcode from pretending it has special knowledge of any particular upstream.
Proposed renames (all in
varcode/phasing.pyandvarcode/__init__.py)IsovarAssemblyProvider(Protocol)ReadPhasingSource(Protocol) +MutantTranscriptSource(Protocol).has_contig(variant, transcript)has_evidence(variant)onReadPhasingSourcevariants_in_contig(variant, transcript)partners_in_cis(variant)onReadPhasingSourcemutant_transcript(variant, transcript)MutantTranscriptSourceIsovarPhaseResolverReadPhaseResolverReadPhaseResolverrequires aReadPhasingSource. The mutant-transcript channel is a separate optional concern — when the wrapped provider also satisfiesMutantTranscriptSource, the resolver routesmutant_transcript(...)calls to it; otherwise that method returnsNone.This is the same protocol split that issue #377 already proposed; this issue subsumes #377 by making the rename fully generic instead of half-renamed. Close #377 in favor of this one.
What happens on the isovar side
Isovar keeps its adapters. They're legitimate — isovar depends on varcode, so it can ship classes that satisfy varcode-defined Protocols:
isovar.IsovarReadPhasing(the light adapter from Add IsovarReadPhasing adapter for RNA-read variant phasing isovar#183) implementsReadPhasingSource. Already shaped correctly; no method renames needed there.isovar.VarcodeAdapter(the heavy one) implements bothReadPhasingSourceandMutantTranscriptSource. Needs minor surgery to match the new method names; also probably worth renaming (drop the "Varcode" prefix) since with this change there's no varcode-named Protocol to adapt to — it's just "adapter from IsovarResult to the generic interfaces." Suggested:IsovarPhasingAndTranscriptProvideror just two classes (IsovarReadPhasing+IsovarMutantTranscript).That's a follow-up isovar PR, blocking on this issue landing.
End-to-end demo (acceptance test)
The varcode side has zero mention of "Isovar" anywhere — including in the imports. Same demo works against any other
ReadPhasingSourceimplementation (long-read tools, hand-rolled stubs, future RNA-phasing packages) without varcode noticing.Scope
ReadPhasingSource+MutantTranscriptSourceProtocols invarcode/phasing.py.IsovarAssemblyProvider.IsovarPhaseResolver→ReadPhaseResolver. Update its constructor to accept anyReadPhasingSource; exposemutant_transcript(...)only when the wrapped source also satisfiesMutantTranscriptSource.tests/test_isovar_phase_resolver.py— rename totests/test_read_phase_resolver.py; renameStubAssemblyProvider→StubReadPhasingSourcewith the new methods.varcode/__init__.pyre-exports.docs/germline.md— replaceIsovarPhaseResolverreferences withReadPhaseResolver. Keep one sentence noting Isovar as the most common implementer.VarcodeAdaptermethods to match new protocols. Consider splittingVarcodeAdapterinto two classes (one per protocol).Out of scope
MutantTranscriptconstruction logic in isovar'sVarcodeAdapterstays in isovar. Varcode doesn't need to know how Isovar builds it — only that something implementsMutantTranscriptSource.mutant_transcript(variant, transcript) -> MutantTranscript | None.IsovarRNAEvidenceResolveretc.) — separate issue if/when those exist.References
IsovarReadPhasingPR. Adapter shape already matchesReadPhasingSource; no change needed to the PR itself once this lands.varcode/phasing.py:34-146— current Isovar-named API.isovar/varcode_adapter.py:29— existing heavy adapter, needs companion update.