Skip to content

[Bioc-devel] annotation() to BiocGenerics?

13 messages · Valerie Obenchain, Benilton Carvalho, Nicolas Delhomme +4 more

Original
Valerie Obenchain
# 
'annotation' and 'annotation<-' have been moved from Biobase (2.17.8) to 
BiocGenerics (0.3.2). This should propagate through tomorrows build.

Valerie
On 08/30/2012 11:35 AM, Valerie Obenchain wrote:
2 days later
Robert Castelo
# 
Dear Valerie and other developers,

executing the vignette from package GSVA fails with an error related to 
the change described in your email (because before it did not appear):

Error: processing vignette ?GSVA.Rnw? failed with diagnostics:
  chunk 19
Error in function (classes, fdef, mtable)  :
   unable to find an inherited method for function "annotation<-", for 
signature "ExpressionSet", "missing"
Execution halted

the main function of the package, gsva(), calls at some point 
'annotation(eset)' for some ExpressionSet object 'eset'.

i've tried the following solutions:

1. put in NAMESPACE

importFrom(Biobase, annotation)

and replace every call to 'annotation()' by 'Biobase::annotation()'

2. put in NAMESPACE

importFrom(BiocGenerics, annotation)

and replace every call to 'annotation()' by 'BiocGenerics::annotation()'


but none of them work, both give still the same error.

i've executed the vignette interactively and this is the traceback() and 
sessionInfo() at the line of code producing the error:

 > gbm_es <- gsva(gbm_eset, brainTxDbSets, mx.diff=FALSE, 
verbose=FALSE)$es.obs
Error in function (classes, fdef, mtable)  :
   unable to find an inherited method for function "annotation<-", for 
signature "ExpressionSet", "missing"
 > traceback()
6: stop("unable to find an inherited method for function \"", fdef at generic,
        "\", for signature ", cnames)
5: function (classes, fdef, mtable)
    {
        methods <- .findInheritedMethods(classes, fdef, mtable)
        if (length(methods) == 1L)
            return(methods[[1L]])
        else if (length(methods) == 0L) {
            cnames <- paste0("\"", sapply(classes, as.character),
                "\"", collapse = ", ")
            stop("unable to find an inherited method for function \"",
                fdef at generic, "\", for signature ", cnames)
        }
        else stop("Internal error in finding inherited methods; didn't 
return a unique method")
    }(list("ExpressionSet", "missing"), function (object, ..., value)
    standardGeneric("annotation<-"), <environment>)
4: Biobase::`annotation<-`(eScoEset, "")
3: .local(expr, gset.idx.list, annotation, ...)
2: gsva(gbm_eset, brainTxDbSets, mx.diff = FALSE, verbose = FALSE)
1: gsva(gbm_eset, brainTxDbSets, mx.diff = FALSE, verbose = FALSE)

whose interpretation goes beyond my expertise, so i hope somebody more 
experienced than me has a hint about the problem :)

cheers,
robert.
On 09/18/2012 08:48 PM, Valerie Obenchain wrote:

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Benilton Carvalho
    

    

      
      #
    

  


  

      
Replace

Biobase::`annotation<-`(eScoEset, "")

by

Biobase::`annotation<-`(eScoEset, value="")

b

        
On 21 September 2012 11:49, Robert Castelo <robert.castelo at upf.edu> wrote:

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

  
  


  


      

    

    

      
      

        


  

        

      Nicolas Delhomme
    

    

      
      #
    

  


  

      
Hi all,

I have the same issue for the easyRNASeq package: 

unable to find an inherited method for function "annotation", for signature "Genome_intervals_stranded"

easyRNASeq uses genomeIntervals (another Bioc package) and due to the mentioned change (moving of the annotation generic), both packages are now partially non-functional.

While I completely support such consolidation, this time, the timeline is very tight. The change was done on the 18th, it took 2 days to propagate, luckily the bug was reported immediately, but it's already friday evening here (21st, CET)  and the API freeze is on the 24th, so if we would play by the rule, there'd just be one day left to fix it (the 24th, can't expect to have someone fixing this at the WE, really). 

This raises a more general concern as there's now really a lot of contributed package in Bioc that depends on the excellent core Bioc API. Would not it make sense to have an earlier freeze of the core Bioc API than just a week before the release? Just to make sure that any effect in downstream contributed packages is timely detected? I'm saying this, because the exact same happened for the 2.10 release 6 months ago. I would not mind sparing that extra bit of stress :-)

I've contacted the maintainer of the genomeIntervals package to see if they can use the BiocGeneric generic instead. 

Robert, it would be good if you were to tell the GSVA developer. And btw, thanks for the tip Benilton!

Cheers,

Nico

---------------------------------------------------------------
Nicolas Delhomme

Genome Biology Computational Support

European Molecular Biology Laboratory

Tel: +49 6221 387 8310
Email: nicolas.delhomme at embl.de
Meyerhofstrasse 1 - Postfach 10.2209
69102 Heidelberg, Germany
---------------------------------------------------------------

        
On Sep 21, 2012, at 12:49 PM, Robert Castelo wrote:

        

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

  
  


  


      

    

    

      
      

        


  

        

      Martin Morgan
    

    

      
      #
    

  


  

      
On 09/21/2012 05:05 AM, Benilton Carvalho wrote:

            

      
      
      
    

        
why not just

    annotation(eScoEset) <- ""

with importFrom(Biobase, "annotation<-") in the NAMESPACE?

Martin

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

        

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Martin Morgan
    

    

      
      #
    

  


  

      
On 09/21/2012 08:37 AM, Nicolas Delhomme wrote:

            

      
      
      
    

        
this doesn't show up in the build report? How can it be reproduced?

            

      
      
      
    

        
we'll clean up our mess, but thanks for the report.

            

      
      
      
    

        
Sorry for the inconvenience; we do try to avoid things that will 
definitely be disruptive.

            

      
      
      
    

        
genomeIntervals should previously have importFrom(Biobase, annotation, 
"annotation<-") in its NAMESPACE.

Martin

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

        

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Robert Castelo
    

    

      
      #
    

  


  

      
hi Martin,

        
On 09/21/2012 05:59 PM, Martin Morgan wrote:

            

      
      
      
    

        
well, my understanding of the general policy to use classes and methods 
defined in other packages is that i should *always* "import" them in the 
NAMESPACE file.

however, somehow i feel more comfortable using this pkg::f() idiom when 
i call something defined somewhere else because just reading the code i 
know immediately where imported things come from.

is there any reason related to performance, correctness or coding style 
by which you think i should not do it?

thanks,
robert.

  
  


  


      

    

    

      
      

        


  

        

      Vincent Carey
    

    

      
      #
    

  


  

      
On Fri, Sep 21, 2012 at 12:26 PM, Robert Castelo <robert.castelo at upf.edu> wrote:

            

      
      
      
    

        
user  system elapsed
  1.109   0.004   1.120

            

      
      
      
    

        
user  system elapsed
  0.010   0.000   0.011

ymmv

            

      
      
      
    

        

      
      
    

  
  


  


      

    

    

      
      

        


  

        

      Robert Castelo
    

    

      
      #
    

  


  

      
On 09/21/2012 06:32 PM, Vincent Carey wrote:

            

      
      
      
    

        
=8-o

i'll take it into account.

thanks!
robert.

  
  


  


      

    

    

      
      

        


  

        

      Martin Morgan
    

    

      
      #
    

  


  

      
On 09/21/2012 09:26 AM, Robert Castelo wrote:

            

      
      
      
    

        
I was more struck by this usage

   eScoEset <- Biobase::`annotation<-`(eScoEset, "")

which I would have written, if following your preferred style,

   Biobase::annotation(eScoEset) <- ""

(I personally would have written annotation(eScoEset) <- "").

There is a performance difference (:: is a function call, and there are 
several symbol look-ups involved) but I doubt that would be a serious 
concern in a seldom-used function (different, though if this were called 
in a large loop).

There is a subtle issue relevant to S4, though the implementation of 
annotation<- shields you from this -- note how the following changes the 
value of 'b' as well as 'a'

 > setClass("A", representation(value="numeric"))
 > a <- b <- new("A", value=1)
 > `slot<-`(a, "value", value=2)
An object of class "A"
Slot "value":
[1] 2

 > b
An object of class "A"
Slot "value":
[1] 2

which does not occur for slot(a, "value") <- 2

For what it's worth, the reason for your code breakage is that the 
signature of annotation<- changed from function(object, value) to 
function(object, ..., value). This is desirable because it allows 
methods to define additional arguments to the replacement function, and 
does not break the replacement method when used as annotation(eScoEset) 
<- "" (replacement methods match the right hand side to the last 
argument) but obviously caused problems for your approach.

Martin

            

      
      
      
    

        

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Nicolas Delhomme
    

    

      
      #
    

  


  

      
Hi Martin,

        
On Sep 21, 2012, at 6:07 PM, Martin Morgan wrote:

        

            

      
      
      
    

        
Not that I've seen. I suppose there are several reason to this, one being that for packages to be hosted on Bioc, they should run R CMD check in less than 5 minutes, which is understandable given the amount of package to be built daily. But this impacts the number of tests one can do. The other (shame on me) being that I would need to spend the time to update these tests and add proper unit tests.

            

      
      
      
    

        
I've just looked up the affected code and it should be straightforward to fix, especially given your additional comments.

            

      
      
      
    

        
I know and I'm extremely grateful for all the excellent work you are all doing. Sincerely it's amazing. And without your infrastructure I could not do much of the research I'm doing, so there's no inconvenience. What I wrote was not a critic (sorry if my frenglish sounded so), it's just that I realized that Bioc is growing steadily and that I could imagine this situation to occur more frequently or affect more packages in the future. Which is why I was thinking that the core packages could have a different release schedule than the contributed packages, since many of these depend on the former. For example the core packages could have deadlines a week earlier than contributed packages. Does that sound sensible?

            

      
      
      
    

        
Well it did not and that is part of the reason why it breaks/broke I believe.

Cheers,

Nico

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

  
  


  


      

    

    
1 day later

    

      
      

        
      

    

    

      
      

        


  

        

      Robert Castelo
    

    

      
      #
    

  


  

      
hi Martin,

        
On 09/21/2012 06:54 PM, Martin Morgan wrote:

            

      
      
      
    

        
yes, you're right, but at the time i wrote this code, about year and a 
half ago -BioC 2.7-, those assignments (Biobase::exprs() <- x or 
Biobase::annotation() <- x) were not working, i was getting an error and 
found that syntatic workaround. now i changed into what you suggest 
(which is indeed much easier to read and write) and it works, so i just 
committed this subtle change.

            

      
      
      
    

        
thanks for the clarification, i didn't know about this rightmost rule on 
replacement methods.

robert.