Next Release: 3.3.2

[thomasp85]

Hi Core team (@hadley, @yutannihilation, @clauswilke, @paleolimbot, @karawoo)

We (me and Hadley) have discussed having a new small release in the not too distant future. This should mainly address regressions in 3.3.0 (probably some issues around bi-directional detection now that it is in the wild), a few performance improvements, and getting the TDD PRs getting folded in.

This means that if we have larger feature PRs we should hold off getting them merged in before a discussion in this thread. If we can avoid working off an RC branch it could be great but if it stands in the way of someones work we'll do that so feature PRs can get merged in...

Please use the new 3.3.1 milestone to tag issues and PRs

[yutannihilation]

Thanks.

If we can avoid working off an RC branch it could be great

Does this mean we are going to release the code based on the current master?

[thomasp85]

If we can avoid merging in big new features the next month, then yes. I feel this is obviously simpler and the RC approach is mostly when we don't want to slow down development while ironing out a big release.

If the need arise we will move to an RC branch but only if necessary

[yutannihilation]

Sure, sounds good. I just wondered we might have already merged big features, but it seems not the case.

But @clauswilke might want #3659 to be merged earlier.

[thomasp85]

No, master is pretty clean atm. There are some big PRs ready, so if there is a strong need to merge them in we’ll switch to the standard RC workflow

[clauswilke]

I'm Ok with not merging #3659 for now. I think a minor release is the right approach. Every time we introduce major features (like we did for 3.3.0), we have to assume some things went wrong and we'll need a bug-fix release shortly after. We should just get into this cadence of always preparing for a minor release after a major release.

I do want to get #3864 done, though. I feel this fell through the cracks and was not completed for 3.3.0.

[thomasp85]

I agree on #3864 - that was an oversight from us

[clauswilke]

I've filed two additional issues (related to contouring) that we should look over/make a decision on before 3.3.1: #3875 #3876

[yutannihilation]

It seems some v3.3.0 milestones are left open. Don't we need to care about them?

https://github.com/tidyverse/ggplot2/milestone/19

[thomasp85]

Hi Team... I've been away from ggplot2 for a while as I've worked on more lower level stuff. I'll be devoting some more time the near future with the goal of getting this patch release out of the door

[thomasp85]

Team. I think we are at the point were there are PRs addressing all the tagged issues for 3.1.1. Can everyone look into reviewing their requested PRs and finishing off their own. If we can get this wrapped up I'll run revdepchecks in next week and (assuming nothing pops up) make a release shortly after

[karawoo]

Thanks @thomasp85! If there's anything that still needs a reviewer feel free to tag me. I know I haven't been very active of late, but am happy to help.

[clauswilke]

I'll try to finish things off over the weekend. I'm going to tag #4004 for 3.3.1, because it would be best to fix it now as part of the general overhaul of contour plotting.

[thomasp85]

All issues has been closed and all PRs merged. I'll start the release process.

Thanks to everyone for helping fix regressions from 3.3.0

[thomasp85]

@clauswilke we have a revdepcheck failure related to the GeomFunction rewrite — can you have a look?

RBesT

• Version: 1.6-0
• Source code: https://github.com/cran/RBesT
• Date/Publication: 2020-03-28 05:50:30 UTC
• Number of recursive dependencies: 121

Run cloud_details(, "RBesT") for more info

Newly broken

• checking examples ... ERROR

  ...
  > 
  > ### ** Examples
  > 
  > ## a beta mixture
  > bm <- mixbeta(weak=c(0.2, 2, 10), inf=c(0.4, 10, 100), inf2=c(0.4, 30, 80))
  > 
  > ## extract the two most informative components
  > bm[[c(2,3)]]
  Univariate beta mixture
  Mixture Components:
    inf   inf2 
  w   0.4   0.4
  a  10.0  30.0
  b 100.0  80.0
  > ## rescaling needed in order to plot
  > plot(bm[[c(2,3),rescale=TRUE]])
  Warning: `data` is not used by stat_function()
  Warning: `data` is not used by stat_function()
  Error in FUN(X[[i]], ...) : object 'g' not found
  Calls: <Anonymous> ... ggplot_build.ggplot -> by_layer -> f -> <Anonymous> -> f -> lapply -> FUN
  Execution halted
  

In both

• checking installed package size ... NOTE

    installed size is 34.5Mb
    sub-directories of 1Mb or more:
      doc    1.9Mb
      libs  31.6Mb
  

• checking for GNU extensions in Makefiles ... NOTE

  GNU make is a SystemRequirements.
  

[clauswilke]

Taking a look. Their code is really cryptic, so it's a bit of a challenge to figure out what's going on.

[thomasp85]

completely agree — let me know if you hit a dead end

[clauswilke]

I think this is the problem, and it'll be easy to fix. May not get to it till tomorrow, though.

library(ggplot2)

df <- data.frame(x = c(-5, 5), f = "2*x+3")

ggplot(df, aes(x, color = f)) +
  stat_function(fun = ~ 2*.x+3)

# this should work
ggplot() +
  stat_function(aes(x, color = f), fun = ~ 2*.x+3, data = df)
#> Warning: `data` is not used by stat_function()
#> Error in FUN(X[[i]], ...): object 'x' not found

^{Created on 2020-05-20 by the reprex package (v0.3.0)}

[thomasp85]

No rush if you haven't got time today — I'll be working on the remaining revdepcheck failures anyway

[clauswilke]

PR #4016 should fix the RBesT issue.

[thomasp85]

@clauswilke there's another geom_function related revdep failure. As far as I can see it the new behaviour is correct, but it brakes two packages. In short, geom_function() errors when some of the aesthetics have another length than 1 because the ensure_nonempty_data data frame has a single row (failure in check_aesthetics(). Can I get you to contemplate whether there is a nice fix for this or if we should ask the maintainers to fix? (maybe we should just not inherit aesthetics?)

CGPfunctions

• Version: 0.6.0
• Source code: https://github.com/cran/CGPfunctions
• URL: https://github.com/ibecav/CGPfunctions
• BugReports: https://github.com/ibecav/CGPfunctions/issues
• Date/Publication: 2020-04-02 14:10:03 UTC
• Number of recursive dependencies: 174

Run cloud_details(, "CGPfunctions") for more info

Newly broken

• checking examples ... ERROR

  ...
  > ### Title: See The Distribution
  > ### Aliases: SeeDist
  > 
  > ### ** Examples
  > 
  > SeeDist(rnorm(100, mean = 100, sd = 20), numbins = 15, var_explain = "A Random Sample")
  Warning: There are 100 modal values displaying just the first 3
  Error: Aesthetics must be either length 1 or the same as the data (1): x
  Backtrace:
       █
    1. └─CGPfunctions::SeeDist(...)
    2.   ├─base::print(p)
    3.   └─ggplot2:::print.ggplot(p)
    4.     ├─ggplot2::ggplot_build(x)
    5.     └─ggplot2:::ggplot_build.ggplot(x)
    6.       └─ggplot2:::by_layer(function(l, d) l$compute_aesthetics(d, plot))
    7.         └─ggplot2:::f(l = layers[[i]], d = data[[i]])
    8.           └─l$compute_aesthetics(d, plot)
    9.             └─ggplot2:::f(..., self = self)
   10.               └─ggplot2:::check_aesthetics(evaled, n)
  Execution halted
  

metagen

• Version: 1.0
• Source code: https://github.com/cran/metagen
• URL: http://00tau.github.io/metagen/
• Date/Publication: 2014-05-27 15:43:54
• Number of recursive dependencies: 68

Run cloud_details(, "metagen") for more info

Newly broken

• checking examples ... ERROR

  ...
  > bcg   <- bcgVaccineData()
  > bcg_y <- bcg$logrisk
  > bcg_d <- bcg$sdiv
  > bcg_s <- bcg$size
  > bcg_x <- cbind(1,bcg$x)
  > p <- plotStudyQfuncPfunc(y=bcg_y, d=bcg_d, x=bcg_x, n=500)
  > p[1] # plot of the q-function
  $plotQ
  Error: Aesthetics must be either length 1 or the same as the data (1): x and y
  Backtrace:
      █
   1. ├─(function (x, ...) ...
   2. └─ggplot2:::print.ggplot(x)
   3.   ├─ggplot2::ggplot_build(x)
   4.   └─ggplot2:::ggplot_build.ggplot(x)
   5.     └─ggplot2:::by_layer(function(l, d) l$compute_aesthetics(d, plot))
   6.       └─ggplot2:::f(l = layers[[i]], d = data[[i]])
   7.         └─l$compute_aesthetics(d, plot)
   8.           └─ggplot2:::f(..., self = self)
   9.             └─ggplot2:::check_aesthetics(evaled, n)
  Execution halted
  

[thomasp85]

I have set a release date two weeks into the future since there are a couple of reverse dependencies that needs minor fixing in their unit tests.

I'll be opening issues in the relevant packages detailing fixes

[thomasp85]

@clauswilke the infer failure turns out to also be related to the new function implementation

infer

• Version: 0.5.1
• Source code: https://github.com/cran/infer
• URL: https://github.com/tidymodels/infer, https://infer.netlify.com/
• BugReports: https://github.com/tidymodels/infer/issues
• Date/Publication: 2019-11-19 10:30:02 UTC
• Number of recursive dependencies: 115

Run cloud_details(, "infer") for more info

Newly broken

• checking tests ... ERROR

    Running ‘testthat.R’
  Running the tests in ‘tests/testthat.R’ failed.
  Last 13 lines of output:
     10. ggplot2:::f(l = layers[[i]], d = data[[i]])
     11. l$compute_aesthetics(d, plot)
     12. ggplot2:::f(..., self = self)
     13. ggplot2:::scales_add_defaults(...)
     14. base::lapply(aesthetics[new_aesthetics], eval_tidy, data = data)
     15. rlang:::FUN(X[[i]], ...)
    
    ══ testthat results  ═══════════════════════════════════════════════════════════
    [ OK: 319 | SKIPPED: 27 | WARNINGS: 1 | FAILED: 3 ]
    1. Error: shade_confidence_interval works (@test-shade_confidence_interval.R#19) 
    2. Error: shade_p_value works (@test-shade_p_value.R#21) 
    3. Error: visualize basic tests (@test-visualize.R#91) 
    
    Error: testthat unit tests failed
    Execution halted
  

It's a bit hidden, but the issue lies in the unexpected theory_curve() function. Again, can you gauge if this is something we can fix or if we should request changes? I've already extended the release so if the best solution is to request fix then we should do that

[clauswilke]

@thomasp85 CGPfunctions uses a pattern such as the following:

x <- rnorm(100)

ggplot() +
  aes(x) +
  geom_density() +
  stat_function(fun = dnorm)

I don't think we should encourage that. It's just wrong. They just need to feed in the data as data frame, like so, and everything works fine:

ggplot(data.frame(x = x)) +
  aes(x) +
  geom_density() +
  stat_function(fun = dnorm)

I'll check the other packages. I'm not sure what the consequence will be of not inheriting aesthetics. It seems right in principle, but it would probably require another revdep check. I'll see if the ggplot2 unit tests pass with that change.

[thomasp85]

I agree with your assessment. If the remaining issues also comes from bad practice we’ll just ask them to change it

[clauswilke]

The next one is interesting. metagen uses a pattern of the form:

qplot(c(0, 1), c(0, 0), geom = "blank") +
  stat_function(fun = dnorm)

So apparently qplot() also feeds in data through the mapping.

[clauswilke]

Not inheriting aesthetics makes this unit test fail:

ggplot2/tests/testthat/test-stat-function.R

Lines 58 to 67 in 323af07

	test_that("works with discrete x", {
	dat <- data_frame(x = c("a", "b"))
	base <- ggplot(dat, aes(x, group = 1)) +
	stat_function(fun = as.numeric, geom = "point", n = 2)
	ret <- layer_data(base)
	expect_equal(ret$x, new_mapped_discrete(1:2))
	expect_equal(ret$y, 1:2)
	})

It's such an obscure test that I wonder whether we can sacrifice this behavior? If somebody actually uses this pattern, they could just turn inheriting aesthetics back on.

[clauswilke]

Actually, here is why we can't just set inherit.aes = FALSE. Surprisingly it doesn't break any unit tests.

library(ggplot2)
df <- data.frame(numbers = 1:100)

ggplot(df, aes(x = numbers)) +
  geom_function(fun = identity, inherit.aes = TRUE)

ggplot(df, aes(x = numbers)) +
  geom_function(fun = identity, inherit.aes = FALSE)

^{Created on 2020-05-25 by the reprex package (v0.3.0)}

[clauswilke]

Ok, here's my summary of what's going on with stat_function():

1. CGPfunctions: Uses a broken pattern. They should fix their code.
2. metagen: Uses an awkward pattern that probably should work but doesn't, most likely due to how qplot() works. I think they should fix their code as well.
3. infer: I checked with the development branch. The only problem I can see is vdiffr doppelgangers that need updating. Once that is done, there is no issue due to stat_function().

I propose to leave the current stat_function() code as is and file issues with all three projects.

[thomasp85]

Sounds great. Can I get you to open issues seeing as you’re the best to describe the problem and a fix? If you are short on time I’ll do it

[clauswilke]

Yes, I can do it, no problem.

[ibecav]

Thank you. I'll correct my mistake. Your release is in about 2 weeks?

[clauswilke]

@ibecav Yes, two weeks.

[ibecav]

Change made, submitted to CRAN and apparently accepted. Thank you again for the heads-up.

[thomasp85]

Just an FYI, I'll be doing Avery minor patch release to CRAN today, fixing the infamous extension site links. This is on request by CRAN. This means that this release will be bumped to 3.3.2

[thomasp85]

I'll begin the release process

[thomasp85]

There are still two issues in the reverse dependencies. One of them says that a fix has been submitted, and the other one is a failure on my part to properly inform them. I'll wait until friday and then release. Thanks for all the patience

[yutannihilation]

Just for information, this change about Rd ref warnings might affect the new submission. r-lib/roxygen2#707 (comment)

[thomasp85]

I tried to submit friday but got hit with the issues @yutannihilation mentioned (they didn't show up when checking on win-level so I thought they might have backtracked...

We will wait until beginning next week when roxygen2 hopefully have a full fix

[clauswilke]

Thomas, what's the status of the current master branch? Is it frozen for release or do you have a release branch and master is open for commits?

[thomasp85]

It is frozen for a day or two more (had hoped for releasing Friday but the doc problems got to me). If you have the need you are free to create an rc branch but it should all be over in the beginning of next week

[yutannihilation]

they didn't show up when checking on win-level so I thought they might have backtracked...

I too hoped so as CRAN daily check also doesn't show any warnings...

[clauswilke]

If you have the need you are free to create an rc branch but it should all be over in the beginning of next week

No problem, I'll refrain from merging anything until after the release. Just wanted to know where we stand.

[yutannihilation]

@thomasp85
After some attempts on #4062, it turned out we need to update utilities-tidy-eval.R in addition to use the dev version of roxygen2. Which of the following do you want?

1. Just merge Test: Fix Rd cross-ref warnings on R-devel #4062
2. Create and merge a separate pull request to update utilities-tidy-eval.R, and regenerate the docs on your local.
3. Both update utilities-tidy-eval.R and regenerate the docs on your local.

[thomasp85]

If #4062 is good to go lets just merge that. We’ll need to figure out a path forward to make sure we don’t regress until roxygen2 has been updated proper

[yutannihilation]

Sure. I think #4062 is ready to merge, if the goal is to pass the CRAN check as there's no warnings anymore. But, since r-lib/roxygen2#1109 is not merged yet, we can wait for a while to see if there's any update on the fix.

[thomasp85]

ggplot2 v3.3.2 has been released. Thank you everyone for your contributions and work on this and for the patience during the release process. It took a bit longer than anticipated due partly to breaking revdeps and changes in CRAN.

We will have a considerable release break now, so at your own leisure it is a good time to work on bigger features or refactors

Have a good weekend!

[thomasp85]

One thing I forgot to mention. With this release I've taken over as the formal maintainer of ggplot2. This does not mark any change at all to the governance structure of our team, but merely reflects that I'm the one taking care of releases and thus has to communicate with CRAN

[paleolimbot]

Well-deserved!

[clauswilke]

Thanks for all your hard work, Thomas!

One issue I wanted to raise for the next release: Should the next version be 4.0 instead of 3.4? With the various major refactorings we have planned, this might be appropriate.

[thomasp85]

I think it is too early to say. My experience tells me that not all planned features will make it so let’s just wait and see what state we’re in come release. I have no problem doing a major version bump if it is indeed warranted