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

ADD: detect donor indicators pipeline #109

Merged
merged 9 commits into from
Nov 8, 2024
Merged

Conversation

cherman2
Copy link
Contributor

@cherman2 cherman2 commented Nov 4, 2024

Adds a pipeline for filtering feature-table down to baseline and donors, and running ancombc to detect important donor features.

import pandas as pd
from qiime2 import Metadata

# TODO: Change Import path.
Copy link
Member

Choose a reason for hiding this comment

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

Is this TODO still relevant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes,

I would love to hear your opinion but the imports are going to change once this massive pr gets merged(#101 ) but I didnt want to base this pr off that pr, so I left a todo

Copy link
Member

Choose a reason for hiding this comment

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

Seems fine. After this PR is merged into main, you ought to be able to merge main into the feature branch for #101 and then make the import path changes in #101.

q2_fmt/_ancombc.py Outdated Show resolved Hide resolved
q2_fmt/_ancombc.py Outdated Show resolved Hide resolved
q2_fmt/_ancombc.py Outdated Show resolved Hide resolved
from q2_fmt._peds import _check_for_time_column, _check_reference_column


def detect_donor_indicators(ctx, table, reference_column, time_column,
Copy link
Member

Choose a reason for hiding this comment

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

There should probably be tests that call this pipeline

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a usage ex!

Comment on lines 46 to 47
"""Checks if column is in the metdata
Checks that column is in the metdata and creates helpful error messages.
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem to be an accurate description of what this does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops

q2_fmt/_ancombc.py Outdated Show resolved Hide resolved
@Oddant1
Copy link
Member

Oddant1 commented Nov 5, 2024

@cherman2 most of these are trivial only like 1 or 2 are real

@Oddant1 Oddant1 assigned cherman2 and unassigned Oddant1 Nov 5, 2024
cherman2 and others added 5 commits November 5, 2024 11:44
@cherman2
Copy link
Contributor Author

cherman2 commented Nov 5, 2024

Hi @Oddant1,
I appreciate your review! I guess this is back at ya! Looks like I am getting a build failure saying it can't find q2-amplicon evan said that this isn't an fmt thing

@cherman2 cherman2 assigned Oddant1 and unassigned cherman2 Nov 5, 2024
Copy link
Member

@Oddant1 Oddant1 left a comment

Choose a reason for hiding this comment

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

lgtm

q2_fmt/_examples.py Outdated Show resolved Hide resolved
Co-authored-by: Evan Bolyen <[email protected]>
@cherman2
Copy link
Contributor Author

cherman2 commented Nov 8, 2024

I think this is ready to merge @Oddant1, Would you merge for me?

@Oddant1 Oddant1 merged commit 1cd4738 into qiime2:dev Nov 8, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

3 participants