search

jbloomlab / dms_tools2

software for the analysis and visualization of deep mutational scanning data
GNU General Public License v3.0
29 stars 20 forks source link

added pseudocount to raw codon counts in `syn_selection_by_codon` #40

Closed fwelsh closed 4 years ago

jbloom commented 4 years ago

@Frances: You should make several changes to this:

  1. Make the value of the pseudocount an optional argument in the function signature, and set a reasonable default. Such as pseudocount=0.5. Then expand the documentation to explain what the pseudocount argument means.

  2. Do not use the pseudocount for the Fisher's exact test. That test should be performed on the raw values as the Fisher's test already works fine with zero counts. The pseudocounts should only be used for computing the odds ratios. Implementation-wise, this probably means doing the Fisher test and then adding the pseudocount. Clearly explain this in the docs.

  3. You'll have to fix so it passes the doctest (right now it fails, see here). Note that if you do point (2) above, you should be changing the odds ratios in the doctest output but not the P-values, since the P-value counts won't change.

jbloom commented 4 years ago

OK, I agree with @skhilton's comments especially about default. @fwelsh, do you want to push that, and then I'll merge.

jbloom commented 4 years ago

@fwelsh: Sorry, one more change I realized that is also needed:

  1. Can you update the version number here to 2.6.3.

  2. Can you add to the CHANGELOG a short explanation of the change. Then I can merge this as the new version.

fwelsh commented 4 years ago

Sorry I didn't notice that, but we should be set now!