Basic implementation of an adapter for locatable. #569
Conversation
| def chrom: String | ||
| def start: Int | ||
| def end: Int | ||
| def name: String |
There was a problem hiding this comment.
Should this be here? Should it have a default = "" or perhaps have type Option[String] = None? Or should we have a NamedLocatable sub-trait?
There was a problem hiding this comment.
I don't think it needs a name at all. Things like Variant are locatable, but don't have a name (could default to id). NamedLocatable seems also overkill until we have sub-classes that could benefit from a common NamedLocatable trait. For the latter, what would NamedLocatable help with? Would we have methods somewhere that operate on name? That seems a bit kludgy until we have examples.
| final def length: Int = getLengthOnReference | ||
|
|
||
| /** Calculates the overlap between two locatables assuming that they do in fact overlap. */ | ||
| def overlap(other: Locatable): Int = CoordMath.getOverlap(start, end, other.getStart, other.getEnd) |
There was a problem hiding this comment.
This should probably be bullet-proofed to check for overlap first or return 0 if they don't overlap.
Codecov Report
@@ Coverage Diff @@
## master #569 +/- ##
==========================================
- Coverage 94.86% 94.72% -0.15%
==========================================
Files 104 105 +1
Lines 5867 5876 +9
Branches 394 417 +23
==========================================
Hits 5566 5566
- Misses 301 310 +9
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
- How about an
enclosesmethod? - It would also be nice to have a scala version of
OverlapDetector, so we have the niceapply/get/updatemethods. I find theaddLhsmethod so ugly. - How about a
toIntervalmethod? - How about a scala version of
Interval?IntervalList?
| def chrom: String | ||
| def start: Int | ||
| def end: Int | ||
| def name: String |
There was a problem hiding this comment.
I don't think it needs a name at all. Things like Variant are locatable, but don't have a name (could default to id). NamedLocatable seems also overkill until we have sub-classes that could benefit from a common NamedLocatable trait. For the latter, what would NamedLocatable help with? Would we have methods somewhere that operate on name? That seems a bit kludgy until we have examples.
|
|
||
| /** Provides a basic comparison that orders on chrom (lexicographically) and then start and end position. */ | ||
| def compare(that: A): Int = { | ||
| var result = this.chrom.compareTo(that.chrom) |
There was a problem hiding this comment.
Should we have a trait LocatableWithSequenceDictionary (I'm bad at naming) that has a def contigIdx method (or val), so that we can have karyotype order versus lexicographical?
|
I like this! I'm using |
This is a draft PR containing a class I wrote elsewhere. Intended for discussion (not [yet] nit-picking). The goal being to standardize how we implement classes in scala that have genomic locations while also adapting them for use in HTSJDK's APIs.