Skip to content

Commit

Permalink
housekeeping (#159)
Browse files Browse the repository at this point in the history
* housekeeping

- spell check
- consistently use markdown syntax in roxygen docs
- style
- vignette chunk label fixes
- minor rephrasing
- update roxygen version
- remove unnecessary pander from suggests
- follow best practices for Rproject

* edits to plot config vignette and sundry

* edits to theme maker vignette

* edits to the TLF workflow vignette

* better chunk options

* fix warnings from ggplot2 3.3.3

* revert ggplot2 version bump; spelling

* edits to box plot vignette

* edits to atom plots vignette

* edits to PK ratio vignette
  • Loading branch information
IndrajeetPatil authored Sep 13, 2021
1 parent f6044eb commit dbaa7a6
Show file tree
Hide file tree
Showing 64 changed files with 1,472 additions and 1,217 deletions.
4 changes: 1 addition & 3 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,12 @@ Imports:
Depends:
R (>= 3.5)
Encoding: UTF-8
RoxygenNote: 7.1.1
RoxygenNote: 7.1.2
Roxygen: list(markdown = TRUE)
LazyData: true
Suggests:
knitr,
rmarkdown,
testthat (>= 2.1.0),
pander,
gridExtra
VignetteBuilder: knitr
Collate:
Expand Down
2 changes: 1 addition & 1 deletion R/aaa-utilities.R
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ parseUpdateAestheticProperty <- function(aestheticProperty, plotConfigurationPro
',aesthetic = "', aestheticProperty, '"))'
)),
# remove the legend of aesthetic if default unmapped aesthetic
parse(text = paste0("if(isIncluded(", aestheticProperty, 'Variable, "legendLabels")){plotObject <- plotObject + ggplot2::guides(', aestheticProperty, " = FALSE)}"))
parse(text = paste0("if(isIncluded(", aestheticProperty, 'Variable, "legendLabels")){plotObject <- plotObject + ggplot2::guides(', aestheticProperty, " = 'none')}"))
)
}

Expand Down
12 changes: 6 additions & 6 deletions R/aggregation-input.R
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@ AggregationInput <- R6::R6Class(
#' @field aggregationDimension dimension of aggregation output
aggregationDimension = NULL,

#' @description Create a new \code{AggregationInput} object
#' @description Create a new `AggregationInput` object
#' @param aggregationFunction list of functions to use for aggregation
#' @param aggregationFunctionName vector of function names
#' that will be used as variable name of the aggregation
#' @param aggregationUnit unit of aggregation output
#' @param aggregationDimension dimension of aggregation output
#' @return A new \code{AggregationInput} object
#' @return A new `AggregationInput` object
initialize = function(aggregationFunction = NULL,
aggregationFunctionName = NULL,
aggregationUnit = NULL,
aggregationDimension = NULL) {
aggregationFunctionName = NULL,
aggregationUnit = NULL,
aggregationDimension = NULL) {
self$aggregationFunction <- aggregationFunction
self$aggregationFunctionName <- aggregationFunctionName
self$aggregationUnit <- aggregationUnit
Expand All @@ -36,7 +36,7 @@ AggregationInput <- R6::R6Class(
predefinedPercentiles <- c(0, 1, 2.5, 5, 10, 15, 20, 25, 50, 75, 80, 85, 90, 95, 97.5, 99, 100)
#' @title tlfStatFunctions
#' @description
#' Bank of predifined functions ready to use by Aggregation methods. Bank defined as Enum.
#' Bank of predefined functions ready to use by Aggregation methods. Bank defined as Enum.
#' To access the function from its name, use match.fun: e.g. testFun <- match.fun("mean-1.96sd")
#' @include enum.R
#' @export
Expand Down
53 changes: 26 additions & 27 deletions R/aggregation-summary.R
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
#' @title AggregationSummary
#' @description R6 class to split a data.frame data into subsets defined by unique combinations of elements
#' in the columns \code{xColumnNames} and \code{groupingColumnNames}.
#' Applies functions defined in \code{aggregationFunctionsVector} to column \code{yColumnNames.}
#' Returns a list of data.frame, one data.frame for each function listed in \code{aggregationFunctionsVector}.
#' Each data.frame in list element is named after the function's corresponding string in \code{aggregationFunctionNames}.
#' in the columns `xColumnNames` and `groupingColumnNames`.
#' Applies functions defined in `aggregationFunctionsVector` to column `yColumnNames.`
#' Returns a list of data.frame, one data.frame for each function listed in `aggregationFunctionsVector`.
#' Each data.frame in list element is named after the function's corresponding string in `aggregationFunctionNames`.
#' The summary statistic column name in each data.frame is the same as the name of the data.frame in the returned list.
#' @export
AggregationSummary <- R6::R6Class(
"AggregationSummary",
public = list(
#' @field data data.frame
data = NULL,
#' @field metaData list of information on \code{data}
#' @field metaData list of information on `data`
metaData = NULL,
#' @field xColumnNames character names of grouping variables
xColumnNames = NULL,
#' @field groupingColumnNames character names of grouping variables
groupingColumnNames = NULL,
#' @field yColumnNames character names of dependent variables (that are grouped)
yColumnNames = NULL,
#' @field aggregationInputsVector list of R6 class \code{AggregationInput} objects
#' @field aggregationInputsVector list of R6 class `AggregationInput` objects
aggregationInputsVector = NULL,
#' @field aggregationFunctionsVector list of functions to use for aggregation
aggregationFunctionsVector = NULL,
Expand All @@ -31,31 +31,31 @@ AggregationSummary <- R6::R6Class(
aggregationDimensionsVector = NULL,
#' @field dfHelper data.frame of aggregated values
dfHelper = NULL,
#' @field metaDataHelper list of information on \code{dfHelper}
#' @field metaDataHelper list of information on `dfHelper`
metaDataHelper = NULL,

#' @description Create a new \code{AggregationSummary} object
#' @description Create a new `AggregationSummary` object
#' @param data data.frame
#' @param metaData list of information on \code{data}
#' @param metaData list of information on `data`
#' @param xColumnNames character names of grouping variables
#' @param groupingColumnNames character names of grouping variables
#' @param yColumnNames character names of dependent variables (that are grouped)
#' @param aggregationInputsVector list of R6 class \code{AggregationInput} objects
#' @param aggregationInputsVector list of R6 class `AggregationInput` objects
#' @param aggregationFunctionsVector list of functions to use for aggregation
#' @param aggregationFunctionNames vector of function names that will be used as variable name of the aggregation
#' @param aggregationUnitsVector character vector of units of aggregation output
#' @param aggregationDimensionsVector character vector of dimensions of aggregation output
#' @return A new \code{AggregationSummary} object
#' @return A new `AggregationSummary` object
initialize = function(data,
metaData = NULL,
xColumnNames = NULL,
groupingColumnNames = NULL,
yColumnNames = NULL,
aggregationInputsVector = NULL,
aggregationFunctionsVector = NULL,
aggregationFunctionNames = NULL,
aggregationUnitsVector = NULL,
aggregationDimensionsVector = NULL) {
metaData = NULL,
xColumnNames = NULL,
groupingColumnNames = NULL,
yColumnNames = NULL,
aggregationInputsVector = NULL,
aggregationFunctionsVector = NULL,
aggregationFunctionNames = NULL,
aggregationUnitsVector = NULL,
aggregationDimensionsVector = NULL) {
self$data <- data
self$metaData <- metaData
self$xColumnNames <- xColumnNames
Expand All @@ -71,8 +71,7 @@ AggregationSummary <- R6::R6Class(
self$aggregationUnitsVector <- append(self$aggregationUnitsVector, aggregationInputValue$aggregationUnit)
self$aggregationDimensionsVector <- append(self$aggregationDimensionsVector, aggregationInputValue$aggregationDimension)
}
}
else {
} else {
self$aggregationFunctionsVector <- c(aggregationFunctionsVector)
self$aggregationFunctionNames <- aggregationFunctionNames
self$aggregationUnitsVector <- aggregationUnitsVector
Expand All @@ -81,7 +80,7 @@ AggregationSummary <- R6::R6Class(
self$generateAggregatedValues()
},

#' @description Apply aggregation functions on \code{x}
#' @description Apply aggregation functions on `x`
#' @param x numeric vector
#' @return A list or vector of aggregated values
applyAggregationFunctions = function(x) {
Expand All @@ -99,23 +98,23 @@ AggregationSummary <- R6::R6Class(

xGroupingCols <- self$data[xGroupingColNames] # Extract grouping columns from dataframe and group them into a list called xGroupingCols

yValuesCol <- self$data[ self$yColumnNames ] # Extract column of values from dataframe
yValuesCol <- self$data[self$yColumnNames] # Extract column of values from dataframe

aggSummaries <- aggregate(yValuesCol, xGroupingCols, function(x) {
res <- self$applyAggregationFunctions(x)
return(res)
})

summaryMatrix <- matrix(aggSummaries[[ self$yColumnNames ]], ncol = length(self$aggregationFunctionsVector))
self$dfHelper <- aggSummaries[ xGroupingColNames ]
summaryMatrix <- matrix(aggSummaries[[self$yColumnNames]], ncol = length(self$aggregationFunctionsVector))
self$dfHelper <- aggSummaries[xGroupingColNames]
self$metaDataHelper <- self$metaData[xGroupingColNames]

for (n in seq(1, length(self$aggregationFunctionNames))) {
dF <- data.frame(summaryMatrix[, n])
colnames(dF)[1] <- self$aggregationFunctionNames[n]
self$dfHelper <- cbind(self$dfHelper, dF)

self$metaDataHelper [[self$aggregationFunctionNames[n]]] <- list(unit = self$aggregationUnitsVector[n], dimension = self$aggregationDimensionsVector[n])
self$metaDataHelper[[self$aggregationFunctionNames[n]]] <- list(unit = self$aggregationUnitsVector[n], dimension = self$aggregationDimensionsVector[n])
}
}
)
Expand Down
Loading

5 comments on commit dbaa7a6

@IndrajeetPatil
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@msevestre Out of curiosity, why does this show up as a single commit in the commit history, when the PR had multiple commits?

@msevestre
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we had followed the git worklow, we would have had the following
1 - one issue (for instance Housekeeping)
2- on branch fixing this issue
3- you can commit as many times as you want to the branch in #2. But the actual history of how you got there is in most cases irrelevant. For example, one could argue that bump ggplot, undo bump ggplot etc... are not needed in the history
4- We squash and merge those commits into one (only when we merge onto develop)./ That way the history on develop in linear and each commit corresponds to an issue being fixed or a feature being implemented

see https://github.com/Open-Systems-Pharmacology/Suite/blob/develop/GIT_WORKFLOW.md

@IndrajeetPatil
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed reply! Makes sense.

Hadn't read the git workflow document, so thanks for sending it along.

@msevestre
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@IndrajeetPatil We might well decide to revise the flow in the future. (specifically all about the rebase stuff etc... is not easy for everyone and can lead to some massive frustrations...)
Do you see any drawback from "squash and merge" ?

@IndrajeetPatil
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you see any drawback from "squash and merge" ?

No, not really. I actually like this cleaner approach!

Please sign in to comment.