-
Notifications
You must be signed in to change notification settings - Fork 95
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
simper function returning an error using example dataset #528
Comments
I cannot replicate this, but copy paste of your message runs smoothly without error. Need more info. |
I can reproduce this with the current CRAN version
|
Works with vegan |
Potential fix: Coerce spcontr <- sapply(seq_len(ncol(comm)),
function(i) c(vegdist(comm[,i,drop=FALSE], "man"))) First tests looks good, but I currently don't have the time for proper testing :( (maybe in a week). I also don't know the root cause and why this slips through the tests... @jarioksa what are your thoughts? |
I tested on github master with no problem. Tested again: github master (2.6-3), permute 0.9-7, R 4.2-1. I am not going to look at this any deeper before tomorrow UTC+3. |
@Edild using |
Here are the details of my system, which returned an error for simper:
R version 4.2.1 (2022-06-23)
Platform: x86_64-apple-darwin17.0 (64-bit)
Running under: macOS Monterey 12.6
However, I tried on another computer (same version of vegan, i.e. 2.6-2) and simper() worked. Here are the system details:
R version 4.1.2 (2021-11-01)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 20.04.4 LTS
Thanks for looking into this and no rush from my end,
Best,
CM
|
The |
That's what's puzzeling me also @jarioksa. Can investigate next week deeper... |
FWIW I cannot replicate this on
|
It seems to be a rare issue! Good luck finding a fix and no rush from my perspective, it was more a FYI than a major hurdle. |
I can reproduce it again:
This happens only with a non-clean session. In my case it's related to an internal package ( |
@Edild Thanks for the further work tracking this down. I had assumed as much: as this isn't reproducible in a clean ( With so many attached we're not going to be able to figure out the specific issue without some narrowing down. It would be good if we could get the @camillemellin Can you provide the output from |
If narrowed it down to a reproducible example
looking deeper into it... |
@Edild Reproducible here now too. Thanks! |
|
More funny things in that package:
Just duplicating code, for adding zeros to the diagonal? IMHO questionable and not maintainable... @gavinsimpson anything we can do about this (except telling users no to load the package after vegan)? |
I haven't quite worked out why yet, but when called with {proxy} loaded and attached, Browse[2]> dimnames
[[1]]
[1] "1" "2" "3" "4" "5" "6" "7" "8" "9" "10" "11" "12" "13" "14" "15"
[16] "16" "17" "18" "19" "20"
[[2]]
NULL while without proxy loaded and attached In the example, we have 20 samples and 30 species, and it is finding something related to samples when it should be finding nothing and that's getting applied to something that has second dimension 30, and hence the error about dimnames. Just a note: if debugging this, be careful with the reproducible example as Use library("vegan")
data(dune, dune.env)
library("proxy")
sim <- simper(dune, dune.env$Management, permutations = 99) |
Got it. {proxy} defines an Browse[1]> proxy:::names.dist
function (x)
attr(x, "Labels")
<bytecode: 0x55932a6fdf88>
<environment: namespace:proxy>
Solutions? I'm not sure what is best but a couple of things spring to mind:
foo <- function(i, comm) {
d <- vegdist(comm[, i, drop = FALSE], method = "man")
attr(d, "Labels") <- NULL
d
}
`names.vegdist` <- function(x) NULL I think this is bad behaviour on the part of {proxy} as they don't own the class I'll prep a PR for the first option and we can discuss this further in the PR... |
Well done! Good snooping and fine analysis! It seems that proxy indeed is a dangerous package that changes the behaviour of standard R in many ways. It seems to over-write R functions What we can do is the fix that @Edild already suggested before the analysis: take care that the distances are not distances, but vectorized with |
I decided to go with |
It seems that this problem was already reported in #513. However, the reporter solved the problem privately and therefore was not solved for everybody else. |
Closed via f0cedc8 |
Thanks for reporting this @camillemellin and for you and @Edild especially for helping track down where the problem lay so we could fix this problem created by {proxy} for everyone. |
No problems at all and well done for finding a fix! |
Hello,
I used to be able to successfully run this function, which now returns an error using the example dataset:
Am I missing something, or is this error due to a package update?
Many thanks for your assistance with this.
CM
The text was updated successfully, but these errors were encountered: