AliYoussef96 / LimROTS

A Hybrid Method Integrating Empirical Bayes and Reproducibility-Optimized Statistics for Robust Analysis of Proteomics and Metabolomics Data
https://aliyoussef96.github.io/LimROTS/
Artistic License 2.0
1 stars 0 forks source link

R file style #13

Closed antagomir closed 3 weeks ago

antagomir commented 1 month ago

Some notes on the coding conventions:

LimROTS function: usually R packages use argument "x", suggest to rename argument data.exp to just "x". It is good to follow standards. Same comment for other functions. The "x" is the typical input argument name.

imports: it is better to be more specific with imports, use importFrom and name the particular function unless you really need the entire package. This will have beneficial downstream effects.

Bioc requires indentations of 4 spaces, it seems to have 2 space indentations now? Isn't this generating warnings in BiocCheck?

LimROTS.r file line 497 has "class(ROTS.output) <- "LimROTS" but I did not find definition of the LimROTS class. Does this exist? It would be not advisable to create a new class, however. It is advisable to use existing Bioconductor class (e.g. SummarizedExperiment) and this is a likely thing to pop in pkg review, too.

I think that inside functions you shouldnt write like "limma::makeContrasts" but instead before function you would do "importFrom limma makeContrasts" and then within the function you would just write "makeContrasts" and same comment for all functions.

Field names in outputs such as this (https://github.com/AliYoussef96/LimROTS/blob/main/R/UPS1.Case4.r) -> better to not use periods, commas, or other special characters in field names. This may cause problems downstream.

When you write "SummarizedExperiment" in roxygen documentation, better to write \code{\link[SummarizedExperiment:SummarizedExperiment-class]{SummarizedExperiment}} etc. Check mia and other packages source code for examples, they also aim to follow recommended coding style.

AliYoussef96 commented 3 weeks ago

All these points have been taken into consideration. The LimROTS class has been removed, and the function now returns a list.