Skip to content

Allow AAP to be non integer#18

Merged
gregorgorjanc merged 2 commits intoAlphaGenes:feat_metafoundersfrom
RosCraddock:feat_metafounders
Oct 22, 2024
Merged

Allow AAP to be non integer#18
gregorgorjanc merged 2 commits intoAlphaGenes:feat_metafoundersfrom
RosCraddock:feat_metafounders

Conversation

@RosCraddock
Copy link
Contributor

Small updates to input the alt_allele_prob_file as a float rather than an integer (which is then turned into a float). This now allows the inputting of decimals such as 0.5, 0.45 which previously caused a valueError.

Changed integer to a float to allow decimal place on input e.g 0.5, 0.45 etc. Previously was forced on input to be an integer (then later turned into a float).
@RosCraddock
Copy link
Contributor Author

Apologies, I forgot to tag. @gregorgorjanc and @XingerTang, please review when you can.

@gregorgorjanc
Copy link
Member

@RosCraddock I don't think we need the nonInteger argument here - we just need to pass the dtype as a float and not as an integer. And dtype should likely be float by default anyhow as we work with probabilities in AlphaPeel. OK?

@XingerTang
Copy link
Contributor

@gregorgorjanc Normally that function only takes sequence and genotype input so it is originally designed for the integer inputs.

@gregorgorjanc
Copy link
Member

I see. Couldn't we then stick to dtype arg only (not adding the nonInteger arg) and if we pass in a np.float type then we avoid using the int() (as done in this PR)

@RosCraddock
Copy link
Contributor Author

@gregorgorjanc you're right! I can make the conditional statement on line 67 using the dtype instead.

Copy link
Member

@gregorgorjanc gregorgorjanc left a comment

Choose a reason for hiding this comment

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

LGTM!

@gregorgorjanc gregorgorjanc merged commit 6e36844 into AlphaGenes:feat_metafounders Oct 22, 2024
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