issue with base:::namespaceImportMethods

[Michael Lawrence-3]

Hi,

I've noticed an issue with S4 methods and namespaces which only arises in
particular, difficult to reproduce configurations. One example is the ggbio
package in Bioconductor, which currently emits these warnings when its
namespace is loaded:

----------------------

library(ggbio)
Loading required package: ggplot2

Attaching package: ‘ggbio’

The following object(s) are masked from ‘package:ggplot2’:

    geom_rect, geom_segment, stat_identity, xlim

Warning messages:
1: found methods to import for function ‘append’ but not the generic itself
2: found methods to import for function ‘as.factor’ but not the generic
itself
3: found methods to import for function ‘as.list’ but not the generic
itself
4: found methods to import for function ‘aggregate’ but not the generic
itself
5: found methods to import for function ‘as.table’ but not the generic
itself
6: found methods to import for function ‘complete.cases’ but not the
generic itself
7: found methods to import for function ‘cor’ but not the generic itself
8: found methods to import for function ‘diff’ but not the generic itself
9: found methods to import for function ‘drop’ but not the generic itself

------------------------

I tracked these warnings down to the behavior of the
base:::namespaceImportMethods, which ends up calling
base:::namespaceImportFrom with arguments that seem to violate the
assumptions made in that function. It looks like base:::namespaceImportFrom
(conditionally) assumes that the "vars", "generics" and "packages"
arguments are parallel vectors. However, base:::namespaceImportMethods can
end up filtering 'vars' so that it no longer parallels the other two. Maybe
I am just misreading the code, but the following patch seems to fix things:

Index: src/library/base/R/namespace.R
===================================================================
--- src/library/base/R/namespace.R      (revision 58917)
+++ src/library/base/R/namespace.R      (working copy)
@@ -930,8 +930,10 @@

 namespaceImportMethods <- function(self, ns, vars) {
     allVars <- character()
+    generics <- character()
+    packages <- character()
     allFuns <- methods:::.getGenerics(ns) # all the methods tables in ns
-    packages <- attr(allFuns, "package")
+    allPackages <- attr(allFuns, "package")
     pkg <- methods:::getPackageName(ns)
     if(!all(vars %in% allFuns)) {
         message(gettextf("No methods found in \"%s\" for requests: %s",
@@ -950,16 +952,23 @@
         ## import methods tables if asked for
         ## or if the corresponding generic was imported
         g <- allFuns[[i]]
+        p <- allPackages[[i]]
         if(exists(g, envir = self, inherits = FALSE) # already imported
            || g %in% vars) { # requested explicitly
-            tbl <- methods:::.TableMetaName(g, packages[[i]])
-            if(is.null(.mergeImportMethods(self, ns, tbl))) # a new
methods
table

+            tbl <- methods:::.TableMetaName(g, p)
+            if(is.null(.mergeImportMethods(self, ns, tbl))) { # a new
methods t
able

                allVars <- c(allVars, tbl) # import it;else, was merged
+               generics <- c(generics, g)
+               packages <- c(packages, p)
+            }
         }
         if(g %in% vars && !exists(g, envir = self, inherits = FALSE)) {
             if(exists(g, envir = ns) &&
-               methods:::is(get(g, envir = ns), "genericFunction"))
+               methods:::is(get(g, envir = ns), "genericFunction")) {
                 allVars <- c(allVars, g)
+                generics <- c(generics, g)
+                packages <- c(packages, p)
+            }
             else { # should be primitive
                 fun <- methods::getFunction(g, mustFind = FALSE, where =
self)
                 if(is.primitive(fun) || methods::is(fun,
"genericFunction")) {}
@@ -970,7 +979,7 @@
             }
         }
     }
-    namespaceImportFrom(self, asNamespace(ns), allVars, allFuns, packages)
+    namespaceImportFrom(self, asNamespace(ns), allVars, generics, packages)
 }

-----------------------------

Thanks for any advice,

Michael

PS: sessionInfo() (yes, ggbio has a LOT of dependencies):

> sessionInfo()
R Under development (unstable) (2012-04-04 r58917)
Platform: x86_64-unknown-linux-gnu (64-bit)

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C
 [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8
 [7] LC_PAPER=C                 LC_NAME=C
 [9] LC_ADDRESS=C               LC_TELEPHONE=C
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

other attached packages:
[1] ggbio_1.2.0         ggplot2_0.9.0       BiocInstaller_1.4.3

loaded via a namespace (and not attached):
 [1] AnnotationDbi_1.18.0    Biobase_2.16.0          BiocGenerics_0.2.0
 [4] biomaRt_2.12.0          Biostrings_2.24.1       biovizBase_1.2.0
 [7] bitops_1.0-4.1          BSgenome_1.24.0         cluster_1.14.2
[10] colorspace_1.1-1        DBI_0.2-5               dichromat_1.2-4
[13] digest_0.5.2            GenomicFeatures_1.8.0   GenomicRanges_1.8.3
[16] grid_2.16.0             gridExtra_0.9           Hmisc_3.9-3
[19] IRanges_1.14.2          lattice_0.20-6          MASS_7.3-17
[22] Matrix_1.0-6            memoise_0.1             munsell_0.3
[25] plyr_1.7.1              proto_0.3-9.2           RColorBrewer_1.0-5
[28] RCurl_1.91-1            reshape2_1.2.1          Rsamtools_1.8.0
[31] RSQLite_0.11.1          rtracklayer_1.16.0      scales_0.2.0
[34] snpStats_1.6.0          splines_2.16.0          stats4_2.16.0
[37] stringr_0.6             survival_2.36-12        tools_2.16.0
[40] VariantAnnotation_1.2.2 XML_3.9-4               zlibbioc_1.2.0

        [[alternative HTML version deleted]]


______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel

[John Chambers-2]

Thanks, Michael.

It looks good.  I committed it to r-devel (rev 58925).

If nothing bad happens, we can merge it into 2.15.0 patched.  (Nothing I
found in CRAN seems to test complicated importMethodsFrom usage.)

John

On 4/5/12 3:35 PM, Michael Lawrence wrote:

> Hi,
>
> I've noticed an issue with S4 methods and namespaces which only arises in
> particular, difficult to reproduce configurations. One example is the ggbio
> package in Bioconductor, which currently emits these warnings when its
> namespace is loaded:
>
> ----------------------
>
> library(ggbio)
> Loading required package: ggplot2
>
> Attaching package: ‘ggbio’
>
> The following object(s) are masked from ‘package:ggplot2’:
>
>      geom_rect, geom_segment, stat_identity, xlim
>
> Warning messages:
> 1: found methods to import for function ‘append’ but not the generic itself
> 2: found methods to import for function ‘as.factor’ but not the generic
> itself
> 3: found methods to import for function ‘as.list’ but not the generic
> itself
> 4: found methods to import for function ‘aggregate’ but not the generic
> itself
> 5: found methods to import for function ‘as.table’ but not the generic
> itself
> 6: found methods to import for function ‘complete.cases’ but not the
> generic itself
> 7: found methods to import for function ‘cor’ but not the generic itself
> 8: found methods to import for function ‘diff’ but not the generic itself
> 9: found methods to import for function ‘drop’ but not the generic itself
>
> ------------------------
>
> I tracked these warnings down to the behavior of the
> base:::namespaceImportMethods, which ends up calling
> base:::namespaceImportFrom with arguments that seem to violate the
> assumptions made in that function. It looks like base:::namespaceImportFrom
> (conditionally) assumes that the "vars", "generics" and "packages"
> arguments are parallel vectors. However, base:::namespaceImportMethods can
> end up filtering 'vars' so that it no longer parallels the other two. Maybe
> I am just misreading the code, but the following patch seems to fix things:
>
> Index: src/library/base/R/namespace.R
> ===================================================================
> --- src/library/base/R/namespace.R      (revision 58917)
> +++ src/library/base/R/namespace.R      (working copy)
> @@ -930,8 +930,10 @@
>
>   namespaceImportMethods<- function(self, ns, vars) {
>       allVars<- character()
> +    generics<- character()
> +    packages<- character()
>       allFuns<- methods:::.getGenerics(ns) # all the methods tables in ns
> -    packages<- attr(allFuns, "package")
> +    allPackages<- attr(allFuns, "package")
>       pkg<- methods:::getPackageName(ns)
>       if(!all(vars %in% allFuns)) {
>           message(gettextf("No methods found in \"%s\" for requests: %s",
> @@ -950,16 +952,23 @@
>           ## import methods tables if asked for
>           ## or if the corresponding generic was imported
>           g<- allFuns[[i]]
> +        p<- allPackages[[i]]
>           if(exists(g, envir = self, inherits = FALSE) # already imported
>              || g %in% vars) { # requested explicitly
> -            tbl<- methods:::.TableMetaName(g, packages[[i]])
> -            if(is.null(.mergeImportMethods(self, ns, tbl))) # a new
> methods
> table
>
> +            tbl<- methods:::.TableMetaName(g, p)
> +            if(is.null(.mergeImportMethods(self, ns, tbl))) { # a new
> methods t
> able
>
>                  allVars<- c(allVars, tbl) # import it;else, was merged
> +               generics<- c(generics, g)
> +               packages<- c(packages, p)
> +            }
>           }
>           if(g %in% vars&&  !exists(g, envir = self, inherits = FALSE)) {
>               if(exists(g, envir = ns)&&
> -               methods:::is(get(g, envir = ns), "genericFunction"))
> +               methods:::is(get(g, envir = ns), "genericFunction")) {
>                   allVars<- c(allVars, g)
> +                generics<- c(generics, g)
> +                packages<- c(packages, p)
> +            }
>               else { # should be primitive
>                   fun<- methods::getFunction(g, mustFind = FALSE, where =
> self)
>                   if(is.primitive(fun) || methods::is(fun,
> "genericFunction")) {}
> @@ -970,7 +979,7 @@
>               }
>           }
>       }
> -    namespaceImportFrom(self, asNamespace(ns), allVars, allFuns, packages)
> +    namespaceImportFrom(self, asNamespace(ns), allVars, generics, packages)
>   }
>
> -----------------------------
>
> Thanks for any advice,
>
> Michael
>
> PS: sessionInfo() (yes, ggbio has a LOT of dependencies):
>
>> sessionInfo()
> R Under development (unstable) (2012-04-04 r58917)
> Platform: x86_64-unknown-linux-gnu (64-bit)
>
> locale:
>   [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C
>   [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8
>   [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8
>   [7] LC_PAPER=C                 LC_NAME=C
>   [9] LC_ADDRESS=C               LC_TELEPHONE=C
> [11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C
>
> attached base packages:
> [1] stats     graphics  grDevices utils     datasets  methods   base
>
> other attached packages:
> [1] ggbio_1.2.0         ggplot2_0.9.0       BiocInstaller_1.4.3
>
> loaded via a namespace (and not attached):
>   [1] AnnotationDbi_1.18.0    Biobase_2.16.0          BiocGenerics_0.2.0
>   [4] biomaRt_2.12.0          Biostrings_2.24.1       biovizBase_1.2.0
>   [7] bitops_1.0-4.1          BSgenome_1.24.0         cluster_1.14.2
> [10] colorspace_1.1-1        DBI_0.2-5               dichromat_1.2-4
> [13] digest_0.5.2            GenomicFeatures_1.8.0   GenomicRanges_1.8.3
> [16] grid_2.16.0             gridExtra_0.9           Hmisc_3.9-3
> [19] IRanges_1.14.2          lattice_0.20-6          MASS_7.3-17
> [22] Matrix_1.0-6            memoise_0.1             munsell_0.3
> [25] plyr_1.7.1              proto_0.3-9.2           RColorBrewer_1.0-5
> [28] RCurl_1.91-1            reshape2_1.2.1          Rsamtools_1.8.0
> [31] RSQLite_0.11.1          rtracklayer_1.16.0      scales_0.2.0
> [34] snpStats_1.6.0          splines_2.16.0          stats4_2.16.0
> [37] stringr_0.6             survival_2.36-12        tools_2.16.0
> [40] VariantAnnotation_1.2.2 XML_3.9-4               zlibbioc_1.2.0
>
> [[alternative HTML version deleted]]
>
>
>
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel