VEuPathDB / microbiomeComputations

1 stars 0 forks source link

add pvalue floor #53

Closed asizemore closed 11 months ago

asizemore commented 11 months ago

Resolves #51

This PR adds a variable P_VALUE_FLOOR to the package. We can use this floor in differential abundance now, and in correlation later. In short, we replace any pvalue smaller than P_VALUE_FLOOR with P_VALUE_FLOOR. The frontend will also get a matching variable with a comment to ensure the two stay linked.

The one wrench in this plan is the adjusted p values. Since we can't just assign one based on the floor, and using the original adjusted p-value doesnt make sense since then there would be two points at P_VALUE_FLOOR with different adjusted p values, I've assigned those modified points to have an adjusted p value of NA. Since the point is to explore, i think a tiny p value should suggest to the user that the point is worth exploring further and on their own. It's not my favorite solution, so other ideas are welcome!

d-callan commented 11 months ago

maybe any time we return pvalues, we should return the pvalue floor used as well? then you dont have to have a separate variable frontend?

for the adjusted pvalue, im not sure i understand.. is assigning one based on the floor any worse than assigning a floor in the first place? the most accurate way to represent these floored values for both the pvalue and adj pvalue would id think be to do something like '<= pvalue' and '<= adj pvalue'

asizemore commented 11 months ago

comments addressed! Pvalue floor is now a prop, the pvaluefloor and adj pvalue floors are returned with the data, and we do a real calculation for the adj pvalue floor.

Please give a close eye to my adj pvalue calculation!

asizemore commented 11 months ago

@d-callan ready for another review!

asizemore commented 11 months ago

@d-callan both comments resolved in the latest commit