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

Additional metrics in estimate_richness #568

Open
colinbrislawn opened this issue Jan 19, 2016 · 9 comments · May be fixed by #575
Open

Additional metrics in estimate_richness #568

colinbrislawn opened this issue Jan 19, 2016 · 9 comments · May be fixed by #575
Labels

Comments

@colinbrislawn
Copy link

Hello Joey,

Thanks for developing this software. It serves as a great model for amplicon analysis and R software development.

A collaborator recently requested alpha diversity metrics of evenness. I added Pielou to compliment Shannon and SimpsonE to compliment InvSimpson. Are you interested in added evenness metrics?

If you are, I'm interested in contributing code. I'd love to hear your feedback.

Colin Brislawn

@joey711
Copy link
Owner

joey711 commented Jan 20, 2016

Sounds great. If you contribute I will attempt to merge. I don't have a doc on contributing, but the unit test examples already included serve as a decent template. Also, if you simply provide working examples in the roxygen doc, especially with expected edge cases, I can make sure they get included.

let me know if you have any questions. I look forward to the pull request.

Cheers!

joey

@colinbrislawn
Copy link
Author

Hello Joey,

When introducing new evenness metrics, I also changed the names of an existing one.

Current       New-name
Simpson       Simpson
InvSimpson    SimpsonD
              SimpsonE

Do you think I should rename InvSimpson or keep it as it is?
(I could also keep both or change other names, etc)

@colinbrislawn colinbrislawn linked a pull request Jan 27, 2016 that will close this issue
@colinbrislawn
Copy link
Author

Hello Joey,

Any advice on running the testthat scripts locally using a modified version of phyloseq? I'd rather test locally instead of pushing everything to travis.

@joey711
Copy link
Owner

joey711 commented Jan 28, 2016

Yeah, I wouldn't change InvSimpson, as it might break users code, though I can see why you want to change it. Simply adding SimpsonE should be fine.

As for running testthat locally, all you need to do is install your version of the package, and then run the script. There are other options, too, as the standard R CMD check phyloseq bash terminal invocation will do the full package check including tests. testthat documentation probably enumerates the many ways you can run the test script, but testthat tests are also interactive, so you can simply run the new test modules that you are adding to verify that they behave as expected.

Hope that helps!

@HSapers
Copy link

HSapers commented Jul 21, 2016

Hi - I'm really interested in using the evenness statistics too - have these been integrated? Thanks!

@ycl6
Copy link

ycl6 commented Jul 12, 2018

It seems Pielou was not implemented yet?

@colinbrislawn
Copy link
Author

I have implemented both Pielou and Simpson's Evenness in PR #575, but it has not yet been merged into the master branch of Phyloseq.

@joey711 or @spholmes let me know what I can do to get these two evenness metrics added. Are we missing unit tests or documentation?

@colinbrislawn
Copy link
Author

Well this PR is definitely stale after 2 years! Hopefully I can refresh it...

Is there still interest in adding these two metrics? 🎼 For the first time in forever, I can work on this PR.

@bstamps
Copy link

bstamps commented Aug 7, 2020

Very much so, yes. If these could be added to the package that would be outstanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants