Deprecating groupCol and blockCol #62
Conversation
…ass and same was done for block
…me was done for block
…was done for block
|
This pull request seems to include only the new argument names but doesn't have a deprecation process. I found this guide (https://contributions.bioconductor.org/deprecation.html) on function and dataset-level deprecations. @LiNk-NY Do you know whether there is a separate instruction on argument-level deprecations? |
vignettes/lefser.Rmd
Outdated
| @@ -31,7 +31,7 @@ The original software utilizes standard statistical significance tests along | |||
| with supplementary tests that incorporate biological consistency and the | |||
| relevance of effects to identity the features (e.g., organisms, clades, OTU, | |||
| genes, or functions) that are most likely to account for differences between | |||
| the two sample groups of interest, referred as ‘classes’. While *LEfSe* is | |||
| the two sample classes of interest, referred as ‘classes’. While *LEfSe* is | |||
R/lefser.R
Outdated
| attr(res_scores, "grp") <- groupCol | ||
| attr(res_scores, "blk") <- blockCol | ||
| attr(res_scores, "grp") <- classCol | ||
| attr(res_scores, "blk") <- subclassCol |
There was a problem hiding this comment.
These attribute names should also change
| groupCol = "GROUP", | ||
| blockCol = NULL, | ||
| classCol = "CLASS", | ||
| subclassCol = NULL, |
There was a problem hiding this comment.
The lefser function shoul dkeep the groupCol and blockCol in the function signature. They should go after the ... similarly to expr.
There was a problem hiding this comment.
@asyakhl
As Sehyun mentioned, there should be a warning for anyone who sets groupCol and blockCol arguments (e.g., if (!missing(groupCol) || !missing(blockCol)) )
|
ping @asyakhl |
| attr(res_scores, "grp") <- classCol | ||
| attr(res_scores, "blk") <- subclassCol | ||
| attr(res_scores, "class") <- classCol | ||
| attr(res_scores, "subclass") <- subclassCol |
There was a problem hiding this comment.
@asyakhl Were these updated in lefsePlotFeat.R ?
There was a problem hiding this comment.
@asyakhl You should .Deprecated the old arguments as expr.
…t-lefserPlotFeat.R
There was a problem hiding this comment.
Thanks Asya! @asyakhl
Other than the man/lefserPlotFeat.Rd, everything looks in order :)
I think the merge commits got a little bit messy.
For next time, use rebase often to pull changes from devel and to keep a linear history.
Please add Ludwig back to the DESCRIPTION file. Thanks!
| groupCol = "GROUP", | ||
| blockCol = NULL, | ||
| classCol = "CLASS", | ||
| subclassCol = NULL, |
There was a problem hiding this comment.
@asyakhl
As Sehyun mentioned, there should be a warning for anyone who sets groupCol and blockCol arguments (e.g., if (!missing(groupCol) || !missing(blockCol)) )
DESCRIPTION
Outdated
| person("Samuel", "Gamboa-Tuz", , "Samuel.Gamboa.Tuz@gmail.com", "ctb"), | ||
| person("Ludwig", "Geistlinger", , "Ludwig.Geistlinger@sph.cuny.edu", "ctb", c(ORCID = "0000-0002-2495-5464")), |
There was a problem hiding this comment.
@asyakhl Why is this being removed?
There was a problem hiding this comment.
Sorry... I was cleaning up to reflect the contributions to the repo and accidentally deleted the line. I'll put it back.
There was a problem hiding this comment.
This is out of scope of the PR
There was a problem hiding this comment.
Maybe incorporated during rebase?
| #' res_subclass <- lefser(zeller14tn_ra, | ||
| #' classCol = "study_condition", | ||
| #' subclassCol = "age_category") | ||
| #' plot_class <- lefsePlotFeat(res_class, res_class$features[[1]]) |
There was a problem hiding this comment.
@asyakhl The correct function name is lefserPlotFeat, not lefsePlotFeat
Replacing groupCol and blockCol with classCol and subclassCol, and other group and block variable names. Also updated man files.