Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updated revdeps #3303

Closed
wants to merge 1 commit into from

Conversation

@topepo
Copy link
Member

commented May 4, 2019

database file

We checked 2622 reverse dependencies (2212 from CRAN + 410 from BioConductor), comparing R CMD check results across CRAN and dev versions of this package.

  • We saw 30 new problems
  • We failed to check 70 packages

Issues with CRAN packages are summarised below.

New problems

(This reports the first line of each new failure)

  • bayesAB
    checking examples ... ERROR

  • breathteststan
    checking examples ... ERROR

  • CSTools
    checking tests ...

  • esmisc
    checking examples ... ERROR

  • fergm
    checking examples ... ERROR

  • fingertipscharts
    checking examples ... ERROR
    checking tests ...

  • ggalt
    checking whether package ‘ggalt’ can be installed ... WARNING

  • ggforce
    checking whether package ‘ggforce’ can be installed ... WARNING

  • ggpol
    checking whether package ‘ggpol’ can be installed ... WARNING

  • ggpolypath
    checking whether package ‘ggpolypath’ can be installed ... WARNING

  • ggsolvencyii
    checking whether package ‘ggsolvencyii’ can be installed ... WARNING

  • ggspatial
    checking whether package ‘ggspatial’ can be installed ... WARNING

  • ggstance
    checking tests ...

  • ggstatsplot
    checking tests ...

  • ggtern
    checking whether package ‘ggtern’ can be installed ... WARNING

  • heatmaply
    checking installed package size ... NOTE

  • HistDAWass
    checking examples ... ERROR

  • incR
    checking examples ... ERROR

  • interflex
    checking examples ... ERROR

  • jcolors
    checking examples ... ERROR

  • lime
    checking tests ...

  • mlr
    checking examples ... ERROR

  • MTLR
    checking tests ...

  • SCORPIUS
    checking tests ...

  • shiny
    checking tests ...

  • ssMousetrack
    checking installed package size ... NOTE
    checking for GNU extensions in Makefiles ... NOTE

  • stats19
    checking tests ...

  • trialr
    checking installed package size ... NOTE
    checking dependencies in R code ... NOTE
    checking for GNU extensions in Makefiles ... NOTE

  • vdiffr
    checking tests ...

  • xpose
    checking examples ... ERROR
    checking tests ...

Failed to check

  • aslib (NA)
  • BACA (NA)
  • BACCT (NA)
  • bamdit (NA)
  • BayesRS (NA)
  • BNSP (NA)
  • BPEC (NA)
  • bsam (NA)
  • BTSPAS (NA)
  • CaliCo (NA)
  • CollapsABEL (NA)
  • colorednoise (NA)
  • crmPack (NA)
  • Crossover (NA)
  • Deducer (NA)
  • DeLorean (NA)
  • DiversityOccupancy (NA)
  • dynfrail (NA)
  • dynr (NA)
  • evoper (NA)
  • ewoc (NA)
  • fingerPro (NA)
  • fsdaR (NA)
  • G2Sd (NA)
  • gastempt (NA)
  • imageData (NA)
  • imbalance (NA)
  • InSilicoVA (NA)
  • jarbes (NA)
  • joineRML (NA)
  • JointAI (NA)
  • likeLTD (NA)
  • lilikoi (NA)
  • llama (NA)
  • LLSR (NA)
  • matchingMarkets (NA)
  • mbgraphic (NA)
  • mcmcabn (NA)
  • mleap (NA)
  • morse (NA)
  • mwaved (NA)
  • NPflow (NA)
  • OpenStreetMap (NA)
  • openVA (NA)
  • petro.One (NA)
  • phase1PRMD (NA)
  • phase1RMD (NA)
  • pimeta (NA)
  • poppr (NA)
  • PortfolioEffectHFT (NA)
  • qdap (NA)
  • RcmdrPlugin.FuzzyClust (NA)
  • Rdrools (NA)
  • rmcfs (NA)
  • robustHD (NA)
  • rpanel (NA)
  • rpf (NA)
  • rrepast (NA)
  • RSCAT (NA)
  • rstanarm (NA)
  • rsvg (NA)
  • RtutoR (NA)
  • SeqFeatR (NA)
  • simmr (NA)
  • sitmo (NA)
  • spcosa (NA)
  • TeachingDemos (NA)
  • vortexR (NA)
  • XLConnect (NA)
  • zooaRchGUI (NA)
@thomasp85

This comment has been minimized.

Copy link
Member

commented May 6, 2019

@paleolimbot can you begin to go through the list of problems and group them into the same apparent root cause. After that we can begin to tackle the issues one by one

@paleolimbot

This comment has been minimized.

Copy link
Collaborator

commented May 6, 2019

This is what I have so far...I can break these up and file individual issues as well if this is not the best place.

geom_ribbon() failure

There is a failure in bayesAB with the use of geom_ribbon() at https://github.com/FrankPortman/bayesAB/blob/master/R/plotDists.R#L38, where ymin and ymax are getting passed as a parameter rather than specified by aes(). This now causes an error in geom_ribbon() (#3235). Interestingly, this doesn't cause errors in other geometries as long as the vectors are the right length, although it mostly does not lead to the correct plot.

ggplot(data.frame(thing = c("a", "b", "c"))) + 
  geom_point(x = c(1, 2, 3), y = 3)
  
ggplot(data.frame(thing = c("a", "b", "c"))) + 
  geom_point(aes(x = c(1, 2, 3), y = 3))

Theme-related failures

Packages breathteststan, esmisc, fergm, fingertipscharts, jcolors, xpose all fail with error

Error in axis.ticks.length.x.bottom %||% axis.ticks.length.x :
  object 'axis.ticks.length.x.bottom' not found

This failure is at https://github.com/tidyverse/ggplot2/blob/master/R/guides-axis.r#L30. This is a result of #2934. All of these packages define complete themes using theme() or depend on packages that do.

rstan does not fail, but defines a default theme in a non-standard way: https://github.com/stan-dev/rstan/blob/develop/rstan/rstan/R/stan_plot_options.R

MTLR does not define a complete theme, but has a .rds file containing a previous theme object that does not match the current ggplot theme objects (since they have all changed).

ggstatsplot tests its custom theme_pie() using numeric indices ( https://github.com/IndrajeetPatil/ggstatsplot/blob/master/tests/testthat/test_theme_ggstatsplot.R#L65 ). The custom theme is created using theme_bw() + theme(...) and is probably fine, but the index of the object they're trying to test has changed since an element was inserted.

vdiffr failure

vdiffr has two test failures as a result of one figure ("Some title"). The figures are created here (in the one of the test packages):

test_that("ggtitle is set correctly", {
  p <- ggplot2::ggplot()
  expect_doppelganger("Some title", p)
  expect_doppelganger("Some other title", p + ggplot2::ggtitle(NULL))
})

I think the code is failing because the plot title has a slightly different height. The diff of the failing plot is here:

Failed doppelganger: some-title (../figs/ggplot/some-title.svg)

< before                                                                        
> after                                                                         
@@ 15,10 / 15,10 @@                                                             
  <rect x='0.00' y='0.00' width='720.00' height='576.00' style='stroke-width: 1.
  07; stroke: #FFFFFF; fill: #FFFFFF;' />                                       
  <defs>                                                                        
<   <clipPath id='cpNS40OHw3MTQuNTJ8NTcwLjUyfDIyLjUy'>                          
>   <clipPath id='cpNS40OHw3MTQuNTJ8NTcwLjUyfDIyLjc3'>                          
<     <rect x='5.48' y='22.52' width='709.04' height='548.00' />                
>     <rect x='5.48' y='22.77' width='709.04' height='547.75' />                
    </clipPath>                                                                 
  </defs>                                                                       
< <rect x='5.48' y='22.52' width='709.04' height='548.00' style='stroke-width: 1
: .07; stroke: none; fill: #FFFFFF;' clip-path='url(#cpNS40OHw3MTQuNTJ8NTcwLjUyf
: DIyLjUy)' />                                                                  
> <rect x='5.48' y='22.77' width='709.04' height='547.75' style='stroke-width: 1
: .07; stroke: none; fill: #FFFFFF;' clip-path='url(#cpNS40OHw3MTQuNTJ8NTcwLjUyf
: DIyLjc3)' />                                                                  
< <rect x='5.48' y='22.52' width='709.04' height='548.00' style='stroke-width: 1
: .07; stroke: #333333;' clip-path='url(#cpNS40OHw3MTQuNTJ8NTcwLjUyfDIyLjUy)' />
> <rect x='5.48' y='22.77' width='709.04' height='547.75' style='stroke-width: 1
: .07; stroke: #333333;' clip-path='url(#cpNS40OHw3MTQuNTJ8NTcwLjUyfDIyLjc3)' />
  <defs>                                                                        
    <clipPath id='cpMC4wMHw3MjAuMDB8NTc2LjAwfDAuMDA='>                          

pathGrob failures

Packages ggalt, ggforce, ggpol, ggpolypath, ggsolvencyii, and ggspatial all fail with

possible error in 'pathGrob(munched$x, munched$y, ': unused argument (pathId = munched$group)

I assume all of these packages are trying to get holes to show up in grid polygons (for ggspatial and ggpolypath I know this for sure). I don't know enough about grid to know whether this is a grid issue, a ggplot2 issue, or a package developer issue.

Still investigating

  • PepsNMR and vidger are related, related to label aesthetic in geom_text()
  • interflex, HistDAWass, mlr are related
  • Have yet to investigate CStools, gastempt, ggstance, heatmaply, incR, lime, OUTRIDER, PathoStat, SCORPIUS, shiny, ssMousetrack, stats19, trialr
@thomasp85

This comment has been minimized.

Copy link
Member

commented May 6, 2019

Let’s just keep it here until all packages have been gone through. I’m suffering a concussion right now so I’m sadly of less help than I anticipated

@paleolimbot

This comment has been minimized.

Copy link
Collaborator

commented May 6, 2019

No problem, feel better!

@clauswilke clauswilke referenced this pull request May 6, 2019

Closed

Release ggplot2 3.2.0 #3285

16 of 17 tasks complete
@clauswilke

This comment has been minimized.

Copy link
Member

commented May 6, 2019

@paleolimbot The vdiffr failures are due to this PR: #2996
which closed this bug: #2687

The package authors will have to fix their vdiffr reference images. However, vdiffr is not run on CRAN, so this should not trigger problems there.

However, this should be stated explicitly in the release notes. I'll open an issue to remind us.

@clauswilke

This comment has been minimized.

Copy link
Member

commented May 6, 2019

The geom_ribbon() problem is related to this conversation:
#3235 (comment)

Maybe we should revisit this one more time and accept ymin and ymax as parameters after all. As much as we generally don't want people feeding in data through parameters, the truth is people insist on doing it and overall I think we've given up enforcing this. (See also: #2694)

@paleolimbot

This comment has been minimized.

Copy link
Collaborator

commented May 6, 2019

Another group of failures:

annotate() data with facets

PepsNMR and vidger fail with:

Error: Aesthetics must be either length 1 or the same as the data (*): label

Both are doing something like this:

library(ggplot2)
data <- data.frame(x = c(1, 2), y = c(1, 2), facet = c("a", "b"))
p <- ggplot(data, aes(x, y)) + geom_point() + facet_grid(vars(facet))
p + annotate("text", x = 1.5, y = 1.5, label = c("label1", "label2"))
#> Error: Aesthetics must be either length 1 or the same as the data (4): label

In 3.1.1 it resulted in this:

I don't have a good reason for this changing since 3.1.1 except maybe #3242 or #3162. Clearly it isn't the desired behaviour.

@clauswilke

This comment has been minimized.

Copy link
Member

commented May 6, 2019

In reading over the annotate() code, I notice that it treats position aesthetics differently from other ones:

library(ggplot2)
data <- data.frame(x = c(1, 2), y = c(1, 2), facet = c("a", "b"))
p <- ggplot(data, aes(x, y)) + geom_point() + facet_grid(vars(facet))
p + annotate("text", x = c(1, 2), y = 1.5, label = "label")

Created on 2019-05-06 by the reprex package (v0.2.1)

Specifically, it only transfers position values into the data frame it creates:

data <- new_data_frame(position, n = max(lengths))

Maybe annotate() should poll the geom about the aesthetics it understands and move all those into the newly generated data frame. For reference, this is how layer() figures out the aesthetics of a geom:

aes_params <- params[intersect(names(params), geom$aesthetics())]

@clauswilke

This comment has been minimized.

Copy link
Member

commented May 6, 2019

I made a new issue for the annotation problem (#3305).

@thomasp85

This comment has been minimized.

Copy link
Member

commented May 6, 2019

Regarding geom_ribbon() I looked into that during the last release cycle for what became 3.1.1. bayesAB is torturing the API and I think we should tell them to stop doing that instead of making our code fit their use

@paleolimbot

This comment has been minimized.

Copy link
Collaborator

commented May 6, 2019

Another set of errors, possibly related to what broke the annotation code for facets:

Something about zero-row data

All of interflex, HistDAWass, and mlr fail, with

Error: Elements must equal the number of rows or 1

but for different reasons. I've simplified the failing code to reprexes:

# interflex
library(ggplot2)
ggplot() +
  annotate("point", x = numeric(0), y = numeric(0), colour = "red")
#> Error: Elements must equal the number of rows or 1

# histDAWass
library(ggplot2)
df <- data.frame(x = c(7, 8.2, 9.2, 10.8, 15))
ggplot(
  df, 
  aes(x = factor(0), 
      ymin = df$x[1], 
      lower = df$x[2], 
      middle = df$x[3], 
      upper = df$x[4], 
      ymax = df$x[5])
  ) + 
  geom_boxplot(stat = "identity")
#> Error: Elements must equal the number of rows or 1

# mlr
library(ggplot2)
ggplot(data.frame(y = 1), aes(factor(1), y = y)) + geom_violin()
#> Warning in max(data$density): no non-missing arguments to max; returning -
#> Inf
#> Error: Elements must equal the number of rows or 1

In 3.1.1, all of these generated very odd output but did so without erroring. These plots also had other layers in the examples, so it's likely they were intended to fail silently.

# interflex
library(ggplot2)
ggplot() +
  annotate("point", x = numeric(0), y = numeric(0), colour = "red")

# histDAWass
library(ggplot2)
df <- data.frame(x = c(7, 8.2, 9.2, 10.8, 15))
ggplot(
  df, 
  aes(x = factor(0), 
      ymin = df$x[1], 
      lower = df$x[2], 
      middle = df$x[3], 
      upper = df$x[4], 
      ymax = df$x[5])
  ) + 
  geom_boxplot(stat = "identity")

# mlr
library(ggplot2)
ggplot(data.frame(y = 1), aes(factor(1), y = y)) + geom_violin()
#> Warning in max(data$density): no non-missing arguments to max; returning -
#> Inf

@paleolimbot

This comment has been minimized.

Copy link
Collaborator

commented May 6, 2019

Last one for today:

different output column ordering between plyr::ddply and ggplot2:::dapply

This causes a test in ggstance to fail, as ggstance still uses plyr::ddply(). The problem is caused by different column ordering resulting in a slightly different random jitter.

df <- data.frame(y = 1:4, by_col = c("a", "b"))
plyr::ddply(df, "by_col", identity)
#>   y by_col
#> 1 1      a
#> 2 3      a
#> 3 2      b
#> 4 4      b
ggplot2:::dapply(df, "by_col", identity)
#>   by_col y
#> 1      a 1
#> 2      a 3
#> 3      b 2
#> 4      b 4
@paleolimbot

This comment has been minimized.

Copy link
Collaborator

commented May 7, 2019

This is the rest of the packages:

argument order problem in join_keys() is reversed from argument order problem in plyr::join.keys()

For incR, there is a plot that facets by date. This was possible in 3.1.1, but is now broken.

library(ggplot2)
df <- data.frame(date = as.Date("2019-01-01"), x = 1, y = 1)
p <- ggplot(df, aes(x, y)) + geom_point()
p + facet_grid(vars(date))
#> Error in scale_apply(layer_data, x_vars, "train", SCALE_X, x_scales):
p + facet_wrap(vars(date))
#> Error in scale_apply(layer_data, x_vars, "train", SCALE_X, x_scales):

This is a "problem" in join_keys() ( https://github.com/tidyverse/ggplot2/blob/master/R/compat-plyr.R#L161 ). It was also a problem in plyr::join.keys(), but the order of the inconsistency was reversed.

df <- data.frame(date = as.Date("2019-01-01"))
df_fct <- data.frame(date = as.factor(df$date))
unlist(ggplot2:::join_keys(df, df_fct, "date"))
#> x y n 
#> 1 1 1
unlist(ggplot2:::join_keys(df_fct, df, "date"))
#> x y n 
#> 2 1 2

unlist(plyr::join.keys(df, df_fct, "date"))
#> x y n 
#> 2 1 2
unlist(plyr::join.keys(df_fct, df, "date"))
#> x y n 
#> 1 1 1

Installation failures

gastempt failed to install in the revdep check. Testing the package on my machine (MacOS) seemed to not generate any failing tests. I can't find any failures related to ggplot2 from the CMD check.

ssMousetrack and trialr failed to install the first time, so all warnings were "newly broken".

Package size failures

heatmaply is just barely at the 5 MB limit and may have been pushed over the edge by the update.

Connectivity failures

lime, OUTRIDER, PathoStat, shiny, stats19. I couldn't install OUTRIDER, and couldn't replicate the failures in problems.md in any of the others.

Unknown failures

SCORPIUS doesn't have any errors on my machine using the github version.

@clauswilke

This comment has been minimized.

Copy link
Member

commented May 7, 2019

@paleolimbot Could you open a separate issue for each remaining group of problems (for which I haven't opened one yet) and tag each with the 3.2.0 milestone? Then we can triage there. Ignore installation failures, package size failures, etc. Cover only the issues that were brought about by some identifiable change in the ggplot2 code.

@paleolimbot

This comment has been minimized.

Copy link
Collaborator

commented May 8, 2019

@clauswilke I think I got them all as separate issues.

I think several were caused because there was ggplot2 code that depended on plyr-specific corner case handling. Is the general approach to fix the ggplot2 code or to fix things in compat-plyr.R so that they match the previous behaviour?

@clauswilke

This comment has been minimized.

Copy link
Member

commented May 8, 2019

I don't think there's a generic answer to how we address these issues. It depends on how many downstream packages are affected, how difficult it would be for us to restore the previous behavior, and how strongly we believe that the new behavior is more appropriate/correct. Contacting the downstream maintainers and telling them that they need to fix something is always an option, but if it's easy for us to prevent a problem then we should rather do that.

@hadley

This comment has been minimized.

Copy link
Member

commented Jun 4, 2019

Replaced by #3352

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.