-
Notifications
You must be signed in to change notification settings - Fork 60
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
ACTIN-292: Add Chromosomal Rearrangements #612
Conversation
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.
The logic looks good to me! Would be good to update the actual README at some point since you do make some assumptions (e.g. 98% deleted -> deletion).
Are you sure that in both cases the "far" arm is the Q arm in both chr1 and chr19? I will check with Charles, also just out of interest
...datamodel/src/main/java/com/hartwig/hmftools/datamodel/purple/ChromosomalRearrangements.java
Outdated
Show resolved
Hide resolved
orange-datamodel/src/main/java/com/hartwig/hmftools/datamodel/purple/PurpleRecord.java
Outdated
Show resolved
Hide resolved
...c/main/java/com/hartwig/hmftools/orange/algo/purple/ChromosomalRearrangementsDeterminer.java
Outdated
Show resolved
Hide resolved
...c/main/java/com/hartwig/hmftools/orange/algo/purple/ChromosomalRearrangementsDeterminer.java
Outdated
Show resolved
Hide resolved
...c/main/java/com/hartwig/hmftools/orange/algo/purple/ChromosomalRearrangementsDeterminer.java
Outdated
Show resolved
Hide resolved
orange/src/main/java/com/hartwig/hmftools/orange/algo/purple/PurpleInterpreter.java
Outdated
Show resolved
Hide resolved
...st/java/com/hartwig/hmftools/orange/algo/purple/ChromosomalRearrangementsDeterminerTest.java
Outdated
Show resolved
Hide resolved
...st/java/com/hartwig/hmftools/orange/algo/purple/ChromosomalRearrangementsDeterminerTest.java
Outdated
Show resolved
Hide resolved
...st/java/com/hartwig/hmftools/orange/algo/purple/ChromosomalRearrangementsDeterminerTest.java
Show resolved
Hide resolved
...st/java/com/hartwig/hmftools/orange/algo/purple/ChromosomalRearrangementsDeterminerTest.java
Show resolved
Hide resolved
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.
Some new minor comments but still approved :)
...st/java/com/hartwig/hmftools/orange/algo/purple/ChromosomalRearrangementsDeterminerTest.java
Outdated
Show resolved
Hide resolved
...st/java/com/hartwig/hmftools/orange/algo/purple/ChromosomalRearrangementsDeterminerTest.java
Outdated
Show resolved
Hide resolved
...st/java/com/hartwig/hmftools/orange/algo/purple/ChromosomalRearrangementsDeterminerTest.java
Outdated
Show resolved
Hide resolved
...st/java/com/hartwig/hmftools/orange/algo/purple/ChromosomalRearrangementsDeterminerTest.java
Outdated
Show resolved
Hide resolved
{ | ||
int centromere1 = refGenomeCoordinates.centromere(CHR_1); | ||
int centromere19 = refGenomeCoordinates.centromere(CHR_19); | ||
int length19QArm = refGenomeCoordinates.length(CHR_19) - centromere19; |
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.
@kduyvesteyn Should there be a "+ 1" added here? Same in line 43.
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.
@kduyvesteyn I made some drawings and looked into how this is calculated in SvUtilities, and figured it out :)
No description provided.