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

Not including chromosome with _ in the name #19

Closed
Adoni5 opened this issue Oct 19, 2023 · 4 comments · Fixed by #20
Closed

Not including chromosome with _ in the name #19

Adoni5 opened this issue Oct 19, 2023 · 4 comments · Fixed by #20

Comments

@Adoni5
Copy link
Contributor

Adoni5 commented Oct 19, 2023

Hi @wdecoster - this is a continuation of #18!

if !chrom.contains('_') {

This line was the guilty party as to why I had empty normalised alignment counts per contig. The problem is the HG38 reference has _ in the contig names (example : NC_000024.10), and I'm not sure why this check was included. What was it's purpose originally? Could it be removed?

Thanks
Rory

@wdecoster
Copy link
Owner

Hi Rory,

I probably wanted to ignore all the extra (unplaced) contigs from the karyotype output and limit the output to the "main" chromosomes, but that is indeed a problem if all chromosomes are like that. Whoops! I should have put some more thought into this.
But this line will have to go then, and users will have to deal with many more small contigs. I can't come up with a way that, without fail, removes the extra contigs.

Wouter

@wdecoster
Copy link
Owner

Not fixed with that PR :)

@wdecoster wdecoster reopened this Oct 19, 2023
@Adoni5
Copy link
Contributor Author

Adoni5 commented Oct 19, 2023

Haha! I figured it would be something like excluding alt contigs. I guess people can just remove them from the reference pre alignment.

I'll open an excellent PR for this and cash in some Hacktoberfest credit

@wdecoster
Copy link
Owner

I guess in a more complicated CLI --karyotype could optionally take a file/list of contigs that the user wants to use for the karyotype, but for now (or forever) this will have to do :)
Thanks again!

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 a pull request may close this issue.

2 participants