-
Notifications
You must be signed in to change notification settings - Fork 309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reference region cleanup #1291
Reference region cleanup #1291
Conversation
Test PASSed. |
new ReferenceRegion(feature.getContigName, feature.getStart, feature.getEnd) | ||
} | ||
|
||
/** | ||
* Builds a reference region for a feature without strand information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without strand information.
→ with strand set.
def stranded(feature: Feature): ReferenceRegion = { | ||
checkFeature(feature) | ||
require(feature.getStrand != null, | ||
"Strand is not defined in feature %s.".format(feature)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add @throws doc for this
@@ -148,7 +148,9 @@ object ReferenceRegion { | |||
* @return The site where this genotype lives. | |||
*/ | |||
def apply(genotype: Genotype): ReferenceRegion = { | |||
ReferenceRegion(genotype.getVariant) | |||
ReferenceRegion(genotype.getContigName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few general questions (not about this line specifically):
Rename ReferenceRegion.orientation
to ReferenceRegion.strand
?
Add strand parameters to other methods (e.g. fromStart
, toEnd
, all
)?
Set strand when copying from one ReferenceRegion
to another (i.e. line 234)?
Resolves bigdatagenomics#1286. After commit 525fda8, we don't set several of the fields in `Variant` when nested in a `Genotype`. Here, we add code to `ReferenceRegion` to pull the fields that are not set in the nested `Variant` from the core `Genotype` fields.
e0a0367
to
361f923
Compare
Thanks for the comments @heuermh. I've rebased and pushed two commits addressing your comments. When this looks good to you, I'll squash commits 2–4 down into a single commit for merge. |
… methods. Resolves bigdatagenomics#1285. Removes `ReferenceRegion.apply` methods for `Feature` and `AlignmentRecord` schemas, and replaces with `stranded` and `unstranded`. Additionally, we change `orientation` to `strand` thoughout Reference{Region, Position} and add strand as an optional argument on the various apply-like methods.
361f923
to
70160d7
Compare
Squashed down. |
Test PASSed. |
Thank you, @fnothaft |
Test PASSed. |
Resolves #1285 and #1286. The first two commits need to be squashed together.