Get rid of warnings and notes in R CMD check

[IndrajeetPatil]

Closes #162

[codecov-commenter]

Codecov Report

Merging #338 (af9bb6c) into develop (25f8fa9) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #338   +/-   ##
========================================
  Coverage    64.21%   64.21%           
========================================
  Files           50       50           
  Lines         2979     2979           
========================================
  Hits          1913     1913           
  Misses        1066     1066           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 25f8fa9...af9bb6c. Read the comment docs.

[Yuri05]

I was trying to understand where those global varibales are defined - and failed.
E.g. "fillLength":

TLF-Library/R/plot-timeprofile.R

Lines 104 to 109 in 87f42c2

	# fillLength defined in .parseUpdateAestheticProperty
	fillValues <- .getAestheticValues(
	n = fillLength,
	selectionKey = plotConfiguration$ribbons$fill,
	aesthetic = "fill"
	)

The comment says "fillLength defined in .parseUpdateAestheticProperty" - but there is no such definition in .parseUpdateAestheticProperty - and also not elsewhere in the code.

TLF-Library/R/aaa-utilities.R

Lines 84 to 98 in 87f42c2

	.parseUpdateAestheticProperty <- function(aestheticProperty, plotConfigurationProperty) {
	c(
	parse(text = paste0(aestheticProperty, 'Variable <- gsub("`", "", mapLabels$', aestheticProperty, ")")),
	parse(text = paste0(aestheticProperty, "Length <- length(unique(mapData[, ", aestheticProperty, "Variable]))")),
	# Update the property using ggplot `scale` functions
	parse(text = paste0(
	"suppressMessages(plotObject <- plotObject + ggplot2::scale_", aestheticProperty, "_manual(",
	"values=.getAestheticValues(n=", aestheticProperty, "Length,",
	"selectionKey=plotConfiguration$", plotConfigurationProperty, "$", aestheticProperty,
	',aesthetic = "', aestheticProperty, '")))'
	)),
	# remove the legend of aesthetic if default unmapped aesthetic
	parse(text = paste0("if(isIncluded(", aestheticProperty, 'Variable, "legendLabels")){plotObject <- plotObject + ggplot2::guides(', aestheticProperty, " = 'none')}"))
	)
	}

I am very confused

[IndrajeetPatil]

I think that comment is outdated.

As mentioned in this NOTE:

plotTimeProfile: no visible binding for global variable 'fillLength'

This unquoted symbol or name appears in the code, but it is not defined anywhere, and so R CMD Check complaints.

This is a common problem with non-standard evaluation, and I am using the standard solution.

[IndrajeetPatil]

For example,

df <- data.frame(x = 1, y = 2)

df[, "x"]

dplyr::select(df, x) # no visible binding for global variable 'x'

[IndrajeetPatil]

Ref: https://r-pkgs.org/check.html#sec-check-checks

[Yuri05]

Well, maybe it's just my very limited understanding of R, but I just don't get how you can use undefined variable? It must be defined somewhere, or not?

Where does the value of fillLength come from in the piece of code above?

[Yuri05]

ok, thx anyway.
@pchelle Can you explain?

[IndrajeetPatil]

IINM, he is on vacation this week.

[pchelle]

Sorry for the delay.
The variable fillLength is created during the call of .parseUpdateAestheticProperty when input argument is "fill"
To prevent potential copy/paste issues of very similar lines of code, most of the functions from aaa-utilities.R directly output R code as character strings (which are then transformed into expressions to be evaluated as actual R code using parse(text) and eval(expression).

Below shows the expression returned when the function .parseUpdateAestheticProperty is called with the arguments "fill" and ribbons.

> .parseUpdateAestheticProperty("fill", "ribbons")

expression(
fillVariable <- gsub("`", "", mapLabels$fill),
fillLength <- length(unique(mapData[, fillVariable])),
suppressMessages(
plotObject <- plotObject + ggplot2::scale_fill_manual(
values = .getAestheticValues(n = fillLength, selectionKey = plotConfiguration$ribbons$fill, aesthetic = "fill")
)
), 
if (isIncluded(fillVariable, "legendLabels")) {
plotObject <- plotObject + ggplot2::guides(fill = "none")
}
)

When the expression is wrapped by eval(), this code is evaluated and

• Creates the variable fillVariable, column name in the data linked to the fill property
• Creates the variable fillLength, number of unique values for the fill property (used for selecting the right number of colors)
• Update the fill properties of the plot plotObject
• Remove the legend if no mapping was actually defined

Note that changing "fill" by any other property name such as "color" or "alpha" would directly create an evaluated R code with all the property names changed to "color" or "alpha"

[pchelle]

I think the warning in R-CMD-Check is from the function plotTimeProfile which needs to manage between the aesthetic properties of errorbars, scatter points, ribbons and lines while potentially sharing the color aesthetic property.

The plotTimeProfile code was leveraging the creation of the fillLength property created by a previous call of .parseUpdateAestheticProperty to merge the fill property of both simulated and observed data.

Since there is no actual written code of fillLength <- length(unique(mapData[, fillVariable])) (because created and run from the expression), the check flags the variable fillLength as not being created before its use.

[Yuri05]

ok, now I at least understand how it works :)
I have further questions/thoughts regarding this, but I will create separate issues for that.
Accepting the PR for now.