search

qiime2 / q2-gneiss

QIIME2 plugin for Gneiss
BSD 3-Clause "New" or "Revised" License
0 stars 15 forks source link

Ditch the composition #48

Closed mortonjt closed 6 years ago

mortonjt commented 6 years ago

This popped up when we were reviewing the documentation updates here: https://github.com/qiime2/docs/pull/346

Basically, the newest changes to include the pseudocount as an option into the ilr-transform commands negates the need for a Composition type.

However, we also need to modify all other methods that consume Composition objects, which includes visualizations.

In this PR, we just have some commands updated to accept FeatureTable[Frequency] with pseudocount options instead.

mortonjt commented 6 years ago

Sorry @ebolyen , @lisa55asil just recognized this issue when we were doing the documentation

ebolyen commented 6 years ago

oh, that makes sense. Does this impact ANCOM?

mortonjt commented 6 years ago

Nope, since there is still a add_pseudocount cmd.

But may want to address with ANCOM later - - will have to discuss with Shyamal

On Wed, Aug 29, 2018, 3:32 PM Evan Bolyen notifications@github.com wrote:

oh, that makes sense. Does this impact ANCOM?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/qiime2/q2-gneiss/pull/48#issuecomment-417128406, or mute the thread https://github.com/notifications/unsubscribe-auth/AD_a3d4eEek1kPioxt0MCm8ZJajvr6lbks5uVxZggaJpZM4WSako .

mortonjt commented 6 years ago

Ok, we have a default pseudocount of 0.5 now ...

Now that we are replacing zeros with pseudocounts, it is not great to have a pseudocount default of 1 since that won't distinguish between singletons or zeros. 0.5 will at least distinguish