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

IMP: ANCOM -> ANCOM-BC tutorial updates #569

Merged
merged 16 commits into from
Jan 17, 2024
Merged

IMP: ANCOM -> ANCOM-BC tutorial updates #569

merged 16 commits into from
Jan 17, 2024

Conversation

lizgehret
Copy link
Member

Replaces ancom with ancombc in PD Mice and Moving Pictures tutorials.

@lizgehret lizgehret changed the title IMP: ancom -> ancombc tutorial updates IMP: ANCOM -> ANCOM-BC tutorial updates Dec 9, 2023
@Oddant1 Oddant1 assigned Oddant1 and gregcaporaso and unassigned Oddant1 Dec 11, 2023
Copy link
Contributor

@cherman2 cherman2 left a comment

Choose a reason for hiding this comment

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

This looks really good to me @lizgehret. You do an incredible job of explaining ancom-bc

source/tutorials/moving-pictures.rst Outdated Show resolved Hide resolved
source/tutorials/moving-pictures.rst Show resolved Hide resolved
source/tutorials/moving-pictures.rst Outdated Show resolved Hide resolved
source/tutorials/pd-mice.rst Show resolved Hide resolved
source/tutorials/pd-mice.rst Outdated Show resolved Hide resolved
source/tutorials/pd-mice.rst Outdated Show resolved Hide resolved
@gregcaporaso
Copy link
Member

@lizgehret, this looks great!

I made a few little edits in the most recent commit.

  1. "Why?" -> "Why might you expect this?" in the moving pics docs. Since it's not necessarily the case that there would be fewer differentially abundant genera than ASVs, I just felt like "Why might..." was a better way to ask this question.
  2. 'donor + genotype' -> 'donor * genotype' - this aligns with how we run the anova earlier in the tutorial, and makes sense because donor and genotype could interact (apologies if I previously suggested using + here).

Also, in the multiple interfaces moving pictures tutorial, the filtering command is rendering strangely:
Screenshot 2024-01-12 at 11 48 36 AM

If you're good with my edits, this is good to merge once we get that rendering issue sorted out.

Copy link
Member

@gregcaporaso gregcaporaso left a comment

Choose a reason for hiding this comment

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

Just see my comment above about the filtering command rendering strangely. I don't need to review this again before merge. Thanks @lizgehret!

@lizgehret
Copy link
Member Author

Thanks @gregcaporaso! Oh yeah, that does look weird - I'll render locally and see what's going on there!

@lizgehret lizgehret assigned lizgehret and unassigned gregcaporaso Jan 12, 2024
@lizgehret
Copy link
Member Author

lizgehret commented Jan 17, 2024

If you're good with my edits, this is good to merge once we get that rendering issue sorted out.

Okay, so for posterity - this has been fixed, and now looks reasonable when rendered locally:
Screen Shot 2024-01-17 at 1 20 29 PM

Once this passes on CI, I'll go ahead and merge. Thanks for the review @gregcaporaso!

@lizgehret lizgehret merged commit 86d0566 into master Jan 17, 2024
2 checks passed
@gregcaporaso
Copy link
Member

Thanks @lizgehret!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

4 participants