rformassspectrometry / Spectra

Low level infrastructure to handle MS spectra
https://rformassspectrometry.github.io/Spectra/
37 stars 25 forks source link

Discussion style plotting arguments #113

Closed sgibb closed 4 years ago

sgibb commented 4 years ago

Currently our plotSpectra* functions have the label* arguments (labelPos, labelAdj, ...)

Even in base R there are at least two paradigms for solving the problem of equally named arguments for multiple functions: legend: box.col, title.col, text.col (similar to our current approach) dendrogram: edgePar (a named list, e.g. edgePar = list(col = "red", ...))

Do we want to keep our current approach:

plotSpectra <- function(...
                        labelCex = 1, labelSrt = 0,
                        labelAdj = NULL, labelPos = NULL,
                        labelOffset = 0.5, ...) {

Or switch to something like

plotSpectra <- function(..., labels.arg = c(cex = 1, srt = 0, offset = 0.5), ...) {

I would prefer the named list approach.

jorainer commented 4 years ago

Thanks @sgibb for the input - and I prefer the current approach :) - which means we have to wait for @lgatto to vote on this.

I in general don't like list parameters because IMHO they are less user friendly and more difficult to define/pass and check. Also, directly naming the parameters makes them more explicit (and it's easier to document.

jorainer commented 4 years ago

@lgatto , could you please also have a look at this issue? I think we should settle this also before submission.

sgibb commented 4 years ago

Submission is done. So we keep it that way.