Update code to work with ggplot2 v 3.4.0

[kylebaron]

Now passing R CMD check with ggplot2 v 3.4.0.

Changes include:

• drop aes_string() and use nse
• drop use of lwd and use linewidth; passing lwd to geom_line() will trigger the warning about using size
• had to re-configure some tests for functions that add layers to plots; I think the functions were still working fine, but there was something different in the gg objects that was preventing me from using expect_equivalent to test
• don't use ..count.. or ..density.. in histograms; using after_stat("count")

[kyleam]

@kylebaron The goal is for this change to stay compatible with (at least) the previous version of ggplot2, correct?

When I test with 3.3.6, I see a failure and several warnings. I believe they all stem from linewidth not being available before ggplot2 3.4.0.

Here's the failure:

Error ('test-dv-pred-ipred.R:23'): dv-pred-ipred basics [PMP-TEST-081]
Error in `dplyr::distinct(data1, colour, linetype, group, linewidth)`: Must use existing variables.
✖ `linewidth` not found in `.data`.
Backtrace:
 1. dplyr::distinct(data1, colour, linetype, group, linewidth)
      at test-dv-pred-ipred.R:23:2
 2. dplyr:::distinct.data.frame(data1, colour, linetype, group, linewidth)
      at dplyr/R/distinct.R:63:2

There are over 240 warnings, but at a glance I think they are all "Ignoring unknown parameters: linewidth".

[kyleam]

drop use of lwd and use linewidth; passing lwd to geom_line() will trigger the warning about using size

Hmm, right, so lwd is an alias for size, triggering the warning. I wonder if the ggplot2 developers forgot to update lwd to map to linewidth. And had they done that, I think it would have actually provided a nice compatibility layer that would have made many changes here unnecessary (i.e. package code that used lwd would just work across the different versions).

In any case, switching away from lwd sounds good, but adjustments would be needed if we want to remain compatible with ggplot2 versions before 3.4.0 (as mentioned in my previous comment).

[kylebaron]

Thanks, @kyleam. I'm thinking that, rather than trying to manage both versions we'd just depend on ggplot2 v 3.4.0. This would be most consistent with the "rip the band-aid off" approach that they are taking and if we coordinate the switch to new ggplot2 with updated pmplots in the same MPN snapshot, then we should be ok with dependencies.

What do you think?

[kyleam]

I'm thinking that, rather than trying to manage both versions we'd just depend on ggplot2 v 3.4.0.

Sure, no objections from me, especially because (unlike pmforest) the kludges needed here would be more extensive.

if we coordinate the switch to new ggplot2 with updated pmplots in the same MPN snapshot, then we should be ok with dependencies.

Yep, MPN-wise we're set. We're going to pin ggplot2 for the next snapshot and maybe one more, so pmplots will need to get pinned too if it requires ggplot2 v3.4.0. That's fine unless there ends up being any other independent pmplots fixes or changes that you would really like to see on MPN sooner than later.