Skip to content
This repository was archived by the owner on Dec 29, 2023. It is now read-only.

Core regression #16

Closed
wants to merge 13 commits into from
Closed

Core regression #16

wants to merge 13 commits into from

Conversation

mortonjt
Copy link
Collaborator

@mortonjt mortonjt commented Sep 1, 2017

This adds a high level wrapper for the ols regression, as originally recommended by @nbokulich . Basically, this performs the pseudocount addition, the ilr transform, and the ols regression given a tree. This is ideal for scenarios with a pre-defined tree, like a phylogenetic tree.

@thermokarst
Copy link
Contributor

Hi @mortonjt, looks like there are a few different failures happening here (linting, missing data, invalid metadata). Let us know if you need some help fixing this up. Thanks!

@thermokarst thermokarst self-assigned this Sep 7, 2017
@thermokarst thermokarst added the stat:DO-NOT-MERGE Please do not merge this until this label has been removed. label Sep 11, 2017
@thermokarst thermokarst removed their assignment Sep 11, 2017
@jairideout
Copy link
Member

Hi @mortonjt! We noticed this PR has been open for awhile -- is this a WIP or can it be closed?

@mortonjt
Copy link
Collaborator Author

mortonjt commented Oct 4, 2017

This should not be closed, because this will provide a means to actually use phylogenetic trees. The way that the tree types are currently structured, this is not possible.

I'm aiming to have this done by the next release.

@mortonjt
Copy link
Collaborator Author

mortonjt commented Oct 4, 2017

Just raised an issue here to make sure that I don't forget. Sorry about the confusion

@jairideout
Copy link
Member

OK we'll keep this open and ignore for now, just ping us when it's ready for review. For the 2017.10 release cycle we'll need the PR ready for review by 10/19. Thanks!

@mortonjt
Copy link
Collaborator Author

mortonjt commented Oct 4, 2017 via email

@mortonjt
Copy link
Collaborator Author

Sorry for the hold up -- I have tested it, and it seems to be working.

I have explicitly changed the types, and changed the name to 'phylogenetic_regression'. While it is still using the same core balance ols method, it explicitly takes a phylogenetic tree as input. This is so that the data handling on the phylogenetic tree (i.e. matching tips, renaming internal nodes) is done within this module and hidden from the user.

This should be ready for review - let me know if there is anything else that needs to be done with this! I'll be pushing out another tutorial immediately after this is in.

@mortonjt
Copy link
Collaborator Author

@jairideout @thermokarst @ebolyen can this still be reviewed to merge into the next release?

@jairideout
Copy link
Member

Yes!

@jairideout jairideout removed the stat:DO-NOT-MERGE Please do not merge this until this label has been removed. label Oct 13, 2017
Changing balance-taxonomy to accept compositions instead of balances
Note. This can only handle binary categories
@mortonjt
Copy link
Collaborator Author

Sorry, accidental pushed in another commit from another PR :/

I'm thinking about since there is filtering / renaming in both the table and the tree, that could be useful to be done in balance-taxonomy. I'm going to push in one more PR to enable that functionality.

@mortonjt
Copy link
Collaborator Author

I'm going to go ahead and close this and open up a new PR. This new PR will fix the problem that we have had with phylogenies, while prompting a much simpler solution.

@mortonjt mortonjt closed this Oct 18, 2017
@mortonjt mortonjt mentioned this pull request Oct 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants