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

ACTIN-292: Add Chromosomal Rearrangements #612

Merged
merged 9 commits into from
Sep 20, 2024
Merged

ACTIN-292: Add Chromosomal Rearrangements #612

merged 9 commits into from
Sep 20, 2024

Conversation

cbruel
Copy link
Contributor

@cbruel cbruel commented Sep 14, 2024

No description provided.

@cbruel cbruel requested a review from kduyvesteyn September 14, 2024 17:52
Copy link
Contributor

@kduyvesteyn kduyvesteyn left a 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

Copy link
Contributor

@kduyvesteyn kduyvesteyn left a 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 :)

{
int centromere1 = refGenomeCoordinates.centromere(CHR_1);
int centromere19 = refGenomeCoordinates.centromere(CHR_19);
int length19QArm = refGenomeCoordinates.length(CHR_19) - centromere19;
Copy link
Contributor Author

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.

Copy link
Contributor Author

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 :)

@cbruel cbruel merged commit c03b0d8 into master Sep 20, 2024
@cbruel cbruel deleted the ACTIN-292 branch September 20, 2024 12:05
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