rformassspectrometry / QFeatures

Quantitative features for mass spectrometry data
https://RforMassSpectrometry.github.io/QFeatures/
25 stars 7 forks source link

All QFeatures functions should add assays? #193

Open cvanderaa opened 1 year ago

cvanderaa commented 1 year ago

Our paradigm for QFeatures is that no data is removed or changed, it is only added. However, there are some functions that replace the assays. Here is a list:

lgatto commented 1 year ago

How would we want to name the new assays? Postfixing filtered and NA after the original names, unless a vector of new names is provided?

cvanderaa commented 1 year ago

Hmm good point. I was thinking let's just do the same as in the other functions. For example, the name defaults to "logAssay" for logTransform(). But this actually would lead to an error if length(i) > 1. I like the idea of postfixing, but shouldn't we then to the same for all functions that add assays?

lgatto commented 1 year ago

Yes, consistent naming convention should be a priority.

What would you prefer - should be use pre/postfixing as default or only support explicit names? If pre/postfixing, then we need a new issue :-)

cvanderaa commented 1 year ago

After thinking, here are my arguments:

  1. In favor of imposing names:

    • Easier to implement and maintain, that is there is no implementation :sweat_smile: ,
    • Postfixing may lead to very long names, eg Run1_NA_log_filtered_norm_imp_aggregated (imagine doing this for the 20 steps of the SCoPE2 workflow)
    • Names often need to be declared anyway, that is if you don't provide name in step 1, you'll often need to provide i in step 2.
  2. In favor of automatic naming:

    • We keep the current behavior, that would otherwise require going through a deprecation process

Conclusion: I would go for option 1, impose explicit names