Skip to content

Support for regular varcode variant instances in read evidence module#87

Merged
timodonnell merged 2 commits intomasterfrom
support-for-regular-varcode-Variant-instances-in-read-evidence
Jun 17, 2015
Merged

Support for regular varcode variant instances in read evidence module#87
timodonnell merged 2 commits intomasterfrom
support-for-regular-varcode-Variant-instances-in-read-evidence

Conversation

@timodonnell
Copy link
Copy Markdown
Contributor

The read evidence module has been in an awkward place since it uses a different Variant class than the rest of varcode, as it uses interbase coordinates whereas the rest of varcode uses 1-based coordinates. (Varcode will eventually migrate entirely to interbase; see #40). This change makes this situation mildly less annoying by having the read evidence module take care of converting regular varcode Variant instances to the needed interase format.

Review on Reviewable

@timodonnell
Copy link
Copy Markdown
Contributor Author

Mind taking a quick perusal of this, @tavinathanson ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: maybe mark this with TODO for easy discovery of TODOs (I should probably put this suggestion in the wiki)

@tavinathanson
Copy link
Copy Markdown
Contributor

@timodonnell I'm somewhat confused by the read_evidence module; for starters, I don't see your version of Variant defined anywhere, or for that matter, code that actually uses the read_evidence module.

Update: Ah, I'm now looking at https://github.com/hammerlab/ovarian-cancer/blob/53c92e7ddc0b81b0864c7d28fd112c75c8747472/projects/pt189/read_evidence_plot.py to get a feel for usage.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused about why variant and locus are combined here?

(In general, I feel like x_or_y isn't good practice, but I don't yet understand why the code wants this in the first place.)

timodonnell added a commit that referenced this pull request Jun 17, 2015
…iant-instances-in-read-evidence

Support for regular varcode variant instances in read evidence module
@timodonnell timodonnell merged commit 6c19c75 into master Jun 17, 2015
@timodonnell timodonnell deleted the support-for-regular-varcode-Variant-instances-in-read-evidence branch June 17, 2015 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants