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

fix: issue warnings instead of errors when trying to check the header of hap files and issue error when output of transform is not provided to simphenotype #254

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

aryarm
Copy link
Member

@aryarm aryarm commented Jun 28, 2024

We were issuing errors in the .hap reader before but it turns out that there are perfectly legitimate reasons for the header to lack an extra field. For example, if a hap file only contains haplotypes (and not repeats) and we try to provide the hap file to simphenotype, which expects both haplotypes and repeats to be declared in the header.

This PR also switches on a message in simphenotype that would warn when the output of transform is not properly passed to simphenotype. Incidentally, I do this all the time 😅 It also converts the warning to an error.

@aryarm aryarm marked this pull request as draft June 28, 2024 17:29
@aryarm aryarm changed the title fix: issue warnings instead of errors when trying to check the header of hap files fix: issue warnings instead of errors when trying to check the header of hap files and issue error when output of transform is not provided to simphenotype Aug 2, 2024
@aryarm aryarm requested a review from mlamkin7 August 2, 2024 20:53
@aryarm aryarm marked this pull request as ready for review August 2, 2024 20:53
@aryarm
Copy link
Member Author

aryarm commented Aug 2, 2024

@mlamkin7 , can I ask you for a review? Do you think this is the right approach to warning the user about these issues?

Copy link
Collaborator

@mlamkin7 mlamkin7 left a comment

Choose a reason for hiding this comment

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

I'm just confused at the change in sim_phenotype but I do think this change makes sense since it's a faulty assumption

@aryarm aryarm merged commit 0226653 into main Aug 7, 2024
17 checks passed
@aryarm aryarm deleted the fix/simpts-header-check branch August 7, 2024 17:37
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