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

Transformation and Median for avgdist #244

Merged
merged 3 commits into from
Aug 6, 2017

Conversation

Microbiology
Copy link
Contributor

I addressed issue #242 and #243 by adding an option to transform the count data and calculate median instead of mean. 😄

The updates pass my R command checks.

@jarioksa
Copy link
Contributor

jarioksa commented Aug 2, 2017

Seems to work like intended. Some minor comments:

  • it may be safe to use meanfun <- match.fun(meanfun) before using the meanfun function.
  • the function takes now extra arguments (...), but these are ignored. You could use them in output <- apply(afunc, 1:2, meanfun, ...) to allow meanfun with extra arguments, such as trim in mean().
  • the use of return is not necessary and we do not have it in vegan unless necessary (i.e., when exiting the function before reaching the end).

@Microbiology
Copy link
Contributor Author

Thanks for the feedback! :)

I made the suggested revisions and updated the pull request.

@jarioksa jarioksa merged commit 082a2c0 into vegandevs:master Aug 6, 2017
@jarioksa
Copy link
Contributor

jarioksa commented Aug 6, 2017

Fine! Merging now. Thanks for your work!

@jarioksa
Copy link
Contributor

jarioksa commented Aug 7, 2017

@Microbiology : I made some maintenance tweaks to avgdist and commited them to master as 8a0e3f0. Please check these.

There are several commits, and the commit message are intended to be so verbose that you can judge the reasoning behind the changes. Please check chose if you disagree with the changes.

@Microbiology
Copy link
Contributor Author

These look great, thanks @jarioksa! :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants