I ended up just changing the methods package to condition on the "verbose" option, instead of "warn", which was entirely inappropriate, after I read the documentation. So it should no longer report these issues, which shouldn't really hurt anything. Defining them in BiocGenerics is merely a convenience.
On Wed, Apr 1, 2015 at 9:39 PM, Michael Lawrence <micha...@gene.com> wrote: > I used my unreleased rgtags package to search for references to > setOldClass in Bioc: > > library(rgtags) > classes <- sub(".*\\.", "", methods(print)) > defs <- do.call(c, lapply(classes, findDefinitions)) > oldClass <- lapply(expr(defs), `[[`, 1L) == quote(setOldClass) > defs[oldClass] > > tagname path line > 1 AsIs BiocGenerics/R/S3-classes-as-S4-classes.R 20 > 2 POSIXlt gmapR/R/GsnapOutput-class.R 10 > 3 dist MLInterfaces/R/AllClasses.R 105 > 4 dist MLInterfaces/inst/oldFiles/INIT.R 15 > 5 dist graph/R/AllClasses.R 45 > 6 dist phyloseq/R/allClasses.R 151 > 7 formula biovizBase/R/facets-method.R 21 > 8 formula geeni/R/gdManager-class.R 8 > 9 function gQTLstats/R/allS4.R 1 > 10 hclust MLInterfaces/inst/oldFiles/classInterfaces.R 16 > 11 hclust chroGPS/R/clusGPS.R 2 > 12 kmeans MLInterfaces/inst/oldFiles/classInterfaces.R 17 > 13 numeric_version AnnotationHub/R/AnnotationHubMetadata-class.R 3 > 14 numeric_version AnnotationHubData/R/AnnotationHubMetadata-class.R 12 > 15 prcomp MLInterfaces/R/clDesign.R 23 > 16 prcomp MLInterfaces/inst/oldFiles/classInterfaces.R 18 > 17 sessionInfo GGtools/R/AllClasses.R 1 > > Note that POSIXlt, formula and function are already defined by the methods > package. I have removed the calls from gmapR and biovizBase, but the > maintainers of the other packages should be notified. It looks like "dist" > was a good choice, because it is in three packages. The rest of the classes > are somewhat specialized. What's the deal with AnnotationHub and > AnnotationHubData? I will run a similar analysis of CRAN. > > > On Wed, Apr 1, 2015 at 5:18 PM, Hervé Pagès <hpa...@fredhutch.org> wrote: > >> On 04/01/2015 05:05 PM, Michael Lawrence wrote: >> >>> So this explains why I wasn't able to figure out how that package was >>> importing graph, and, yes, I also thought it was strange that graph did >>> not export it. The methods package explicitly conditions on the warn >>> level, so it is apparently intentional. It just looks in the global >>> class table for duplicates, so it does not pay attention to the >>> namespace. It's not clear to what extent the methods package assumes >>> that there are no duplicates in the class table; probably too much work >>> to fix. >>> >>> As for sharing class definitions, perhaps the methods package should >>> define it. It already defines classes from the stats package, like >>> "aov". We could start moving more stuff from BiocGenerics to methods. >>> >> >> Sounds good. The more upstream these class definitions are the better. >> >> Just moved setOldClass("dist") from graph to BiocGenerics 0.13.11 and >> exported the dist class. >> >> H. >> >> >>> >>> >>> On Wed, Apr 1, 2015 at 4:29 PM, Martin Morgan <mtmor...@fredhutch.org >>> <mailto:mtmor...@fredhutch.org>> wrote: >>> >>> On 04/01/2015 03:52 PM, Hervé Pagès wrote: >>> >>> Hi, >>> >>> In the same way that we avoid having 2 packages define the same >>> S4 generic function by moving the shared generic definitions to >>> BiocGenerics, it seems that we should also avoid having 2 >>> packages >>> call setOldClass on the same S3 class. Like with S4 generic >>> functions, >>> we've already started to do this by putting some setOldClass >>> statements in BiocGenerics (e.g. we've done it for the >>> 'connection' >>> classes 'file', 'url', 'gzfile', 'bzfile', etc..., see >>> class?gzfile). >>> So if nobody objects we'll do this for the 'dist' class too. >>> >>> Then you won't need to use setOldClass in your cogena package >>> Zhilong. >>> You'll just need to make sure that you import BiocGenerics. >>> >>> >>> This sounds like an ok work-around to me. >>> >>> For the hard-core... >>> >>> One thing is that this is not seen when the package is loaded by >>> itself, e.g., >>> >>> > biocLite("zhilongjia/cogena") >>> > library(cogena) >>> > >>> >>> only when loaded by BiocCheck (on the source directory) >>> >>> > BiocCheck::BiocCheck("cogena") >>> * This is BiocCheck, version 1.3.13. >>> * BiocCheck is a work in progress. Output and severity of issues may >>> change. >>> * Installing package... >>> Note: the specification for S3 class "dist" in package 'cogena' >>> seems equivalent to one from package 'graph': not turning on >>> duplicate class definitions for this class. >>> ^C >>> >>> This is because BiocCheck (indirectly?) Imports: graph. But the old >>> class definition seems to 'leak' (even though graph is not on the >>> search path, and the dist old class is not exported from graph, and >>> BiocCheck doesn't import the non-exported dist class, and cogena >>> doesn't Depend or Import graph!) >>> >>> Also of interest perhaps is that the Note is only printed when >>> warn=1 (which BiocCheck also uses) >>> >>> (new R session) >>> >>> > requireNamespace("graph") >>> Loading required namespace: graph >>> > requireNamespace("cogena") >>> Loading required namespace: cogena >>> > q() >>> >>> (new R session:) >>> >>> > options(warn=1) >>> > requireNamespace("graph") >>> Loading required namespace: graph >>> > requireNamespace("cogena") >>> Loading required namespace: cogena >>> >>> Note: the specification for S3 class "dist" in package 'cogena' >>> seems equivalent to one from package 'graph': not turning on >>> duplicate class definitions for this class. >>> > >>> >>> >>> >>> Cheers, >>> H. >>> >>> >>> On 04/01/2015 03:28 PM, Michael Lawrence wrote: >>> >>> Using setOldClass is generally fine. In this case, the graph >>> package is >>> already defining the dist class, so you could just import >>> that. The graph >>> package might have to export it though. >>> >>> On Wed, Apr 1, 2015 at 3:15 PM, Zhilong Jia >>> <zhilong...@gmail.com <mailto:zhilong...@gmail.com>> wrote: >>> >>> Hi, >>> >>> Here is the package. >>> (https://tracker.bioconductor.__org/issue1204 >>> <https://tracker.bioconductor.org/issue1204> or >>> https://github.com/zhilongjia/__cogena >>> <https://github.com/zhilongjia/cogena>; ). When I >>> biocCheck it, there is a >>> note. >>> >>> Note: the specification for S3 class “dist” in package >>> ‘cogena’ seems >>> equivalent to one from package ‘graph’: not turning on >>> duplicate class >>> definitions for this class. >>> >>> >>> In the source code, there are two R files are related >>> with this issue, >>> cogena_class.R >>> and >>> <https://github.com/__zhilongjia/cogena/blob/master/ >>> __R/cogena_class.R >>> <https://github.com/zhilongjia/cogena/blob/master/ >>> R/cogena_class.R>> >>> dist_class.R >>> <https://github.com/__zhilongjia/cogena/blob/master/ >>> __R/dist_class.R >>> <https://github.com/zhilongjia/cogena/blob/master/ >>> R/dist_class.R>> >>> in the R >>> dir. Here there is a dist slot in cogena class. In the >>> dist_class.R >>> <https://github.com/__zhilongjia/cogena/blob/master/ >>> __R/dist_class.R >>> <https://github.com/zhilongjia/cogena/blob/master/ >>> R/dist_class.R>>, >>> I use >>> setOldClass, but it seems it is not recommended by >>> Bioconductor. >>> >>> How to repair this issue? Thank you. >>> >>> Regards, >>> Zhilong >>> <https://github.com/__zhilongjia/cogena/blob/master/ >>> __R/cogena_class.R >>> <https://github.com/zhilongjia/cogena/blob/master/ >>> R/cogena_class.R>> >>> >>> [[alternative HTML version deleted]] >>> >>> _________________________________________________ >>> Bioc-devel@r-project.org >>> <mailto:Bioc-devel@r-project.org> mailing list >>> https://stat.ethz.ch/mailman/__listinfo/bioc-devel >>> <https://stat.ethz.ch/mailman/listinfo/bioc-devel> >>> >>> >>> [[alternative HTML version deleted]] >>> >>> _________________________________________________ >>> Bioc-devel@r-project.org <mailto:Bioc-devel@r-project.org> >>> mailing list >>> https://stat.ethz.ch/mailman/__listinfo/bioc-devel >>> <https://stat.ethz.ch/mailman/listinfo/bioc-devel> >>> >>> >>> >>> >>> -- >>> Computational Biology / Fred Hutchinson Cancer Research Center >>> 1100 Fairview Ave. N. >>> PO Box 19024 Seattle, WA 98109 >>> >>> Location: Arnold Building M1 B861 >>> Phone: (206) 667-2793 <tel:%28206%29%20667-2793> >>> >>> >>> >> -- >> Hervé Pagès >> >> Program in Computational Biology >> Division of Public Health Sciences >> Fred Hutchinson Cancer Research Center >> 1100 Fairview Ave. N, M1-B514 >> P.O. Box 19024 >> Seattle, WA 98109-1024 >> >> E-mail: hpa...@fredhutch.org >> Phone: (206) 667-5791 >> Fax: (206) 667-1319 >> > > [[alternative HTML version deleted]] _______________________________________________ Bioc-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/bioc-devel