rformassspectrometry / MetaboCoreUtils

Core utilities for metabolomics.
https://rformassspectrometry.github.io/MetaboCoreUtils/index.html
7 stars 6 forks source link

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

Closed sgibb closed 11 months ago

sgibb commented 11 months ago

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 https://github.com/rformassspectrometry/MetaboCoreUtils/blob/7294eecf45a46e873c296edee8eb5e4af3c04ea0/R/chemFormula.R#L55-L75 (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 commented 11 months ago

I add (currently failing) unit tests in the issue63-containsElements-returns-TRUE-for-invalid-elements branch. https://github.com/rformassspectrometry/MetaboCoreUtils/compare/issue63-containsElements-returns-TRUE-for-invalid-elements?expand=1

jorainer commented 11 months ago

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

sgibb commented 11 months ago

fixed by #65