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

New error triggered in compute_group #1826

Closed
aphalo opened this Issue Oct 5, 2016 · 14 comments

Comments

Projects
None yet
4 participants
@aphalo

aphalo commented Oct 5, 2016

In both 'ggpmisc' and 'ggspectra' I was getting a rather cryptic error triggered in my compute_group functions.

Error in as.vector(y) : attempt to apply non-function

With the following traceback

4. as.vector(y) 
3. setdiff(names(mapping), c(geom$aesthetics(), stat$aesthetics())) at layer.r#114
2. ggplot2::layer(stat = StatDebugGroup, data = data, mapping = mapping, 
    geom = geom, position = position, show.legend = show.legend, 
    inherit.aes = inherit.aes, params = list(na.rm = na.rm, summary.fun = summary.fun, 
        summary.fun.args = summary.fun.args, ...)) at stat-debug-group.R#54
1.
stat_debug_group() 

It can be easily fixed by adding force(data) as the first statement in my compute_group function. I haven't seen this error the previous time I tested with 'ggplot2' from Github (some weeks ago).

This does not necessarily need a fix, but thought good to document it here in case someone else faces some problem.

@aphalo

This comment has been minimized.

aphalo commented Oct 5, 2016

I should have told above, that this error is triggered only in some stats, apparently depending on the code of the compute function.

@hadley

This comment has been minimized.

Member

hadley commented Oct 5, 2016

Could you please try and create a minimal reprex that doesn't use any ggpmisc code?

@hadley hadley added bug reprex labels Oct 5, 2016

@hadley hadley added this to the v2.2.0 milestone Oct 5, 2016

@aphalo

This comment has been minimized.

aphalo commented Oct 6, 2016

I will try to make a reprex during the coming weekend. This is rather tricky as I am unable to set breakpoints in RStudio in the compute group function and adding a print(data) statement removes the error in the same way as force(data) does. Preparing a reprex will be just a question of trial and error unless you have a suggestion for how to more directly trace the problem. I get related errors for several different stats in two different packages, but I cannot yet see a pattern across these stats that gives a hint of what is going on. Some of them are roughly based on the stat_summary code from ggplot2 2.1.0 or maybe 2.0.0. They do not use make_summary_fun() as in ggplot2 2.2.0.

@aphalo

This comment has been minimized.

aphalo commented Oct 6, 2016

Having ggplot2 in depends: and importing the whole package seems to also solve the problem. Or maybe updating to scales 0.4.0.9002 helped as this happened automatically when I installed ggplot2 from Github today. I will not have time to explore this in more depth until the weekend.

@hadley

This comment has been minimized.

Member

hadley commented Oct 7, 2016

It's possible that simply rebuilding your package might've fixed the problem. Could you please check to see if that's the case?

@aphalo

This comment has been minimized.

aphalo commented Oct 8, 2016

Yes, this seems to be the case. I had started suspecting the same yesterday. The problem is that if built with the ggplot2 version from Github my package triggers errors when run if the CRAN version of ggplot2 is installed... So, the problem really only affects the binary. This complicates installation from binaries. I can release new versions of my packages and make them depend on ggplot2 2.2.0. I will be able to submit them to CRAN only after 2.2.0 has been released. Or is there any better option?

@aphalo

This comment has been minimized.

aphalo commented Oct 8, 2016

I am not sure but I suspect that this is also the case for 'ggrepel': under some circumstances it triggers an error that vanishes when ggrepel is rebuilt or installed from sources.
'ggtern' is failing with a different error: object new_panel not found, which is not cured by a clean build of ggtern under ggplot2 from Github.

@aphalo

This comment has been minimized.

aphalo commented Oct 8, 2016

The errors reported by your automatic checks for my other packages photobiologyPlants and photobiologyInOut were just caused by package ggspectra being used in their vignettes.

@hadley

This comment has been minimized.

Member

hadley commented Oct 8, 2016

I think the problem is with the way ggproto works. I'll need to talk this over with @wch to confirm. We might be able to avoid the problem just by asking CRAN to rebuild on release.

@wch

This comment has been minimized.

Member

wch commented Oct 10, 2016

After some discussion with @hadley, I think the source of the problem is that ggproto's inheritance is implemented by saving the superclass object. When ggproto object in package B (call it B::b) inherits from an object in package A (call it A::a), the B::b object saves A::a in a field named super. This is a problem when package B is built with one version of A, and then A is upgraded -- B::b will still have the old version of A::a, but if that old version calls functions in A that have changed or no longer exist, problems will happen.

This is the line in question:
https://github.com/hadley/ggplot2/blob/adc1150/R/ggproto.r#L50

I think it should be possible to do this:

  • In ggproto(), save inherit_ as an unevaluated expression, instead of the object itself.
  • Change e$super to a function that evals and returns inherit_. We would also want to save the calling environment of ggproto() so that the expression gets eval'ed in the right place.
    `R
    e$super <- function() {

    The baseenv() arg speeds up eval a tiny bit

    eval(inherit, parent_env, baseenv())
    }
    ``
  • Redefine $.super: if super is a ggproto object instead of a function, then the current ggproto object is in an extension package built with ggplot2 <= 2.1.0. Throw an informative error in this case.

hadley added a commit that referenced this issue Oct 24, 2016

@hadley hadley closed this in #1872 Oct 24, 2016

hadley added a commit that referenced this issue Oct 24, 2016

@lgautier

This comment has been minimized.

lgautier commented Nov 19, 2016

@aphalo

'ggtern' is failing with a different error: object new_panel not found, which is not cured by a clean build of ggtern under ggplot2 from Github.

I am experiencing this with latest R (3.3.2-patched) and ggplot2 (2.2.0). Do you know whether the fix for issue #1872 was meant to fix it ?

@aphalo

This comment has been minimized.

aphalo commented Nov 22, 2016

@lgautier No it was not meant to fix it. 'ggtern' package's code needs an update before it will work. I do not know how big a rewrite is needed, but ggtern has code modifying ggplot2's internal functions, and redefines generics like ggplot. One of these functions has apparently been removed or renamed in ggplot. I think the maintainer plans an update but I have no idea when he will do it. The git repository for ggtern is in Bitbucket at https://bitbucket.org/nicholasehamilton/ggtern. Most recent commit was 2 months ago. Issue tracking is not enabled. To learn more you need to contact the maintainer by e-mail or through a web form at http://www.ggtern.com/.

@lgautier

This comment has been minimized.

lgautier commented Nov 23, 2016

@aphalo Thanks. I am asking the maintainer whether issue tracking could be enabled.

@aphalo

This comment has been minimized.

aphalo commented Nov 23, 2016

@lgautier He made several commits today. Looks like he will soon submit the update to CRAN.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.