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

betadisper with NAs not passing R CMD check under R Devel #192

Closed
gavinsimpson opened this issue Aug 21, 2016 · 4 comments

Comments

@gavinsimpson
Copy link
Contributor

commented Aug 21, 2016

Kurt Hornik reports that under R Devel on CRAN that betadisper() is throwing an error on the examples in ?betadisper, in particular when the groups and distances have NAs:

> mod2 <- betadisper(dis, groups) ## warnings
Warning in betadisper(dis, groups) :
  missing observations due to 'd' removed
Error in optim(apply(X, 2, median, na.rm = TRUE), fn = medfun, gr = dmedfun, :
  non-finite value supplied by optim
Calls: betadisper -> spatialMed -> ordimedian -> optim

There is clearly something wrong as the code doesn't even recognise that there are missing values in group yet there are.

@gavinsimpson gavinsimpson added the bug label Aug 21, 2016

@gavinsimpson gavinsimpson self-assigned this Aug 21, 2016

gavinsimpson added a commit that referenced this issue Aug 21, 2016
gavinsimpson added a commit that referenced this issue Aug 21, 2016
@gavinsimpson

This comment has been minimized.

Copy link
Contributor Author

commented Aug 21, 2016

This is now fixed in devel version of vegan. I just need to cherry pick the set of commits into the current CRAN branch.

@jarioksa

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2016

There are droplevels in centroids.cca, envfit.default and factorfit. Are they infected?

I think that most of changes could be merged to 2.4-1. I'll have a look at merges next week.

@gavinsimpson

This comment has been minimized.

Copy link
Contributor Author

commented Aug 21, 2016

If droplevels is used on factor with NAs, these will get removed unless we set exclude = NA in the call to droplevels. I haven't looked at those other functions yet but unless we're doing something subsequent testing for missingness in factors whose levels have been dropped, we should be OK. If we want to preserve behaviour prior to this recent change we should probably add exclude = NA as before this change droplevels was defined as factor(x, ...) and hence we got factor(x, ..., exclude = NA) as that is the default. Now we will get factor(x, ..., exclude = NULL) which works weirdly if the factor contains NA.

To be safe we probably should be explicit about excluding NA... so droplevels is truly no-op.

@jarioksa

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2016

Got to check these functions plus other functions which use factor() or as.factor(). They are many (anosim, clamtest, dispweight, factorfit, hierParseFormula, kendall, meandist (explicit factor(grouping, exclude = NULL)), mrpp, permatfull, permatswap, and some plot functions).

@gavinsimpson gavinsimpson referenced this issue Aug 21, 2016
7 of 7 tasks complete

@gavinsimpson gavinsimpson added this to the 2.4-1 milestone Aug 21, 2016

jarioksa added a commit that referenced this issue Aug 30, 2016
implement immediate fix for #192
(cherry picked from commit 515fe50)

@jarioksa jarioksa closed this Sep 7, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.