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

Add tr gwas #26

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

Add tr gwas #26

wants to merge 112 commits into from

Conversation

nicholema
Copy link
Collaborator

add tr-gwas script and wdl. It enables running tr gwas genome wide on AoU cloud environment

Copy link
Contributor

@gymreklab gymreklab left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this PR!

I made mostly minor comments. Once those are addressed we can merge.

See in particular the comment on automatic filtering of covariates - this can be addressed in the next PR but should be high on our list to avoid accidental filtering of covariates we didn't mean to in future runs.

data.to_csv(covar_file_path, sep="\t", index=False)

print(f"Done converting {args.phenotype} to plink format")
return plink_pheno,data
Copy link
Contributor

Choose a reason for hiding this comment

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

don't need to return these since they are not used. can remove this line

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi, when I removed return line, it still output the two files we need but with this error, cuz return is None
plink_pheno,data = main()
TypeError: cannot unpack non-iterable NoneType object

docker/plink2/convert_phenotype_plink.py Show resolved Hide resolved
docker/plink2/convert_phenotype_plink.py Show resolved Hide resolved
docker/plink2/convert_phenotype_plink.py Outdated Show resolved Hide resolved
docker/plink2/convert_phenotype_plink.py Outdated Show resolved Hide resolved
tr-gwas/README.md Show resolved Hide resolved
tr-gwas/tr_gwas.wdl Outdated Show resolved Hide resolved
tr-gwas/tr_gwas.wdl Outdated Show resolved Hide resolved
tr-gwas/tr_gwas.wdl Show resolved Hide resolved
tr-gwas/tr_gwas_aou.py Show resolved Hide resolved
@nicholema
Copy link
Collaborator Author

Thank you for all the comments! I made the changes suggested in the comments. If everything looks good I will confirm merge.

@aryarm aryarm removed their request for review December 2, 2024 20:35
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