rformassspectrometry / MetaboCoreUtils

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

containElements returns TRUE for invalid elements #66

Closed sgibb closed 1 year ago

sgibb commented 1 year ago

This PR fixes #63.

containsElements now returns NA for NA, and FALSE for invalid elements (like "Z").

I am not sure about the return value for invalid elements. IMHO NA would be fine (in that case this PR is superfluous).

If this PR is needed, we could argue about the use of suppressWarnings inside containsElements. containsElements("H2O", "Z") will return FALSE and produce a warning about "Z". (If we ignore this PR the result would be NA anyway).

jorainer commented 1 year ago

Hm, I don't see any added value by returning FALSE if wrong formulas/elements are provided. To me returning NA would make most sense - so I assume we close this PR?

sgibb commented 1 year ago

I agree, closed in favour of the behaviour introduced in #65 .