Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

containsElements("H2O", "Z") returns TRUE #63

Closed
sgibb opened this issue Aug 2, 2023 · 3 comments
Closed

containsElements("H2O", "Z") returns TRUE #63

sgibb opened this issue Aug 2, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@sgibb
Copy link
Member

sgibb commented Aug 2, 2023

containsElements returns TRUE (with a warning) for characters that are not valid elements:

library("MetaboCoreUtils")

containsElements("H2O", "Z")
#> Warning in (function (xx, rr) : The following names are not valid and are
#> dropped: 1
#> [1] TRUE
containsElements("H2O", "J")
#> Warning in (function (xx, rr) : The following names are not valid and are
#> dropped: 1
#> [1] TRUE
containsElements("H2O", "")
#> Warning in (function (xx, rr) : The following names are not valid and are
#> dropped: 1
#> [1] TRUE

Created on 2023-08-02 with reprex v2.0.2

It should return FALSE (without a warning).

It would be possible to add something like

if (length(rr) == 1 && rr < 0)
    return(integer(0))

in the mapply of countElements

mapply(function(xx, rr) {
n <- length(rr)
start <- attr(rr, "capture.start")
end <- start + attr(rr, "capture.length") - 1L
sbstr <- substring(xx, start, end)
## set elements without a number in the formula to one
sbstr[!nchar(sbstr)] <- 1L
sl <- seq_len(n)
nm <- sbstr[sl]
r <- setNames(as.integer(sbstr[n + sl]), nm)
valid <- .isValidElementName(nm)
if (any(!valid)) {
warning(
"The following names are not valid and are dropped: ",
paste0(names(r)[!valid], collapse = ", ")
)
r <- r[valid]
}
r
}, xx = x, rr = rx, SIMPLIFY = FALSE, USE.NAMES = TRUE)
(after the if (is.na(xx)) statement introduced in #62 ) and test for length(x) == 0 in containsElements but that would break many unit tests in test_adducts.R (.pasteElements throws an error because of an unnamed argument instead of silently return ""; even returning a named integer(0) results in many other failures in test_adducts.R).

@sgibb sgibb added the bug Something isn't working label Aug 2, 2023
sgibb added a commit that referenced this issue Aug 2, 2023
sgibb added a commit that referenced this issue Aug 2, 2023
@sgibb
Copy link
Member Author

sgibb commented Aug 2, 2023

@jorainer
Copy link
Member

jorainer commented Aug 4, 2023

Thanks for adding this issue! Can you maybe work on that?

@sgibb
Copy link
Member Author

sgibb commented Aug 9, 2023

fixed by #65

@sgibb sgibb closed this as completed Aug 9, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants