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

use chrX and chrY to match sex chromosomes #713

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

CuriousTim
Copy link
Contributor

The ploidy estimation script uses string comparison to "X" and "Y" to identify sex chromsomes, but the ploidy matrix input to the script uses "chrX" and "chrY" for the sex chromsomes. This commit updates the script to correctly identify sex chrosomomes.

The ploidy estimation script uses string comparison to "X" and "Y" to
identify sex chromsomes, but the ploidy matrix input to the script uses
"chrX" and "chrY" for the sex chromsomes. This commit updates the script
to correctly identify sex chrosomomes.
Copy link
Collaborator

@mwalker174 mwalker174 left a comment

Choose a reason for hiding this comment

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

Thanks this looks okay. Could have left in X and Y in also, but we don't really support hg19 anyway.

@epiercehoffman
Copy link
Collaborator

In a few places in the pipeline we provide chrX and chrY as inputs for this reason (example). Should we do this more consistently?

@mwalker174
Copy link
Collaborator

It's a better solution to have them as inputs but we only support hg38 anyway. Up to @CuriousTim if he wants to do that and re-test it.

@CuriousTim
Copy link
Contributor Author

I can add an input to the script. Should I also update the WDLs?

@mwalker174
Copy link
Collaborator

mwalker174 commented Sep 17, 2024

Yes please update the WDLs and include the allosome contig names as inputs. Have "chrX" and "chrY" as defaults.

Update the ploidy estimation script and associated WDLs to accept a
custom set of allosome contigs, with the default being chrX and chrY.
@CuriousTim
Copy link
Contributor Author

Sometimes the allosomes are given as a FASTA index like this, sometimes as a chrX input like the previously linked example and sometimes it is given as an array like this. Is there a preference for one method?

@mwalker174
Copy link
Collaborator

Sometimes the allosomes are given as a FASTA index like this, sometimes as a chrX input like the previously linked example and sometimes it is given as an array like this. Is there a preference for one method?

I think I'd prefer something like this where we explicitly pass X and Y as separate optional variables.

Expose separate inputs to specify the allosomal contig names in
PloidyEstimation with the defaults being "chrX" and "chrY".
The method to pass the names of the allosomes to PloidyEstimation was
changed from passing a file to passing two strings, but some calling
workflows were not updated to reflect the change. This commit fixes the
inputs.
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