Skip to content

Quotes around nucleotides in Variant representation#139

Merged
iskandr merged 5 commits intomasterfrom
quotes-around-nucleotides
Mar 15, 2016
Merged

Quotes around nucleotides in Variant representation#139
iskandr merged 5 commits intomasterfrom
quotes-around-nucleotides

Conversation

@iskandr
Copy link
Copy Markdown
Contributor

@iskandr iskandr commented Mar 14, 2016

Instead of representing empty strings with ., instead using single quotes around all string fields of a Variant. This brings us closer to having a string representation of a variant which can be evaluated by Python to get back to the same object (the reference name isn't quite there).

Also, got rid of Variant.preserves_reading_frame since I've come around to @arahuja's opinion that this is a confusing property.


This change is Review on Reviewable

Comment thread varcode/variant.py
self.start,
self.ref if self.ref else ".",
self.alt if self.alt else ".",
self.ref,
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.

does this need to check for ref or alt being None or does that never happen?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We check the length of both alt and ref in the initializer so I don't think it's possible for them to be None in a constructed Variant.

@arahuja
Copy link
Copy Markdown
Contributor

arahuja commented Mar 14, 2016

LGTM, one question on whether the alt/ref should still be explicitly set as empty string

iskandr added a commit that referenced this pull request Mar 15, 2016
Quotes around nucleotides in Variant representation
@iskandr iskandr merged commit 19d1684 into master Mar 15, 2016
@iskandr iskandr deleted the quotes-around-nucleotides branch March 15, 2016 16:37
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