Skip to content
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

Added AdamContext.referenceLengthFromCigar #263

Conversation

tdanford
Copy link
Contributor

@tdanford tdanford commented Jun 8, 2014

Adds a new method which has some utility (for some of our code):

ADAMContext.referenceLengthFromCigar, which calculates the 'length along the reference sequence' of an alignment record from its Cigar string. This is useful because the 'rec.start + referenceLengthFromCigar(rec.cigar)' is a natural 'end' value which can be used to calculate whether an ADAMRecord overlaps a given region along the reference.

From this, we can calculate overlap queries on ADAMRecords without having to go through the (somewhat cumbersome) ReferenceMapping framework.

Thoughts? Does this duplicate code that's already somewhere else in ADAM? Also, I'm not parsing the Cigar strings using Picard but rather defining my own quick-and-dirty regex for them, I don't know how people feel about that.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/358/

* reference genome (i.e. skipping clipping, padding, and insertion operators)
*
* @param cigar The CIGAR string whose reference length is to be measured
* @return A non-negative integer, the some of the MDNX= operators in the CIGAR string.
Copy link
Member

Choose a reason for hiding this comment

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

sum, not some ;)

@fnothaft
Copy link
Member

fnothaft commented Jun 8, 2014

Looks good, but I think this'd be a better fit in RichADAMRecord, or in a Util class.

@tdanford
Copy link
Contributor Author

tdanford commented Jun 8, 2014

Good call, I'll move it.

@tdanford
Copy link
Contributor Author

tdanford commented Jun 9, 2014

I fixed those two issues, Frank -- moved it to RichADAMRecord, and fixed the spelling mistake :-)

@fnothaft
Copy link
Member

fnothaft commented Jun 9, 2014

Can you rebase this? Also, do you still want to change the imports in ADAMContext?

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/360/

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/361/

@fnothaft
Copy link
Member

fnothaft commented Jun 9, 2014

@tdanford can you remove the changes to ADAMContext? Then, I'll merge.

The new referenceLengthFromCigar method (and its associated ADAMRecord method, 'referenceLength')
is an implementation of a method which counts the length of an aligned read against the reference
sequence. This is useful because rec.start + rec.referenceLength is a logical "end" coordinate
to be used for testing overlap of an ADAMRecord against a reference range.
@tdanford
Copy link
Contributor Author

tdanford commented Jun 9, 2014

@fnothaft Done. I don't think the Jenkins build has kicked off yet though...

@fnothaft
Copy link
Member

fnothaft commented Jun 9, 2014

Thanks @tdanford! I will wait for the build and merge if it is clean.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/363/

fnothaft added a commit that referenced this pull request Jun 9, 2014
…mCigar

Added AdamContext.referenceLengthFromCigar
@fnothaft fnothaft merged commit 12ab6fc into bigdatagenomics:master Jun 9, 2014
@fnothaft
Copy link
Member

fnothaft commented Jun 9, 2014

Merged! Thanks @tdanford!

@tdanford tdanford deleted the tdanford/referenceLengthFromCigar branch June 9, 2014 20:22
@tdanford
Copy link
Contributor Author

tdanford commented Jun 9, 2014

Thanks, Frank!

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.

3 participants