[Bioc-devel] issue about S4 slot has a dist object.
Michael Lawrence
lawrence.michael at gene.com
Thu Apr 2 06:39:36 CEST 2015
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 <hpages at 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 <mtmorgan at fredhutch.org
>> <mailto:mtmorgan at 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
>> <zhilongjia at gmail.com <mailto:zhilongjia at 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 at r-project.org
>> <mailto:Bioc-devel at 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 at r-project.org <mailto:Bioc-devel at 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: hpages at fredhutch.org
> Phone: (206) 667-5791
> Fax: (206) 667-1319
>
[[alternative HTML version deleted]]
More information about the Bioc-devel
mailing list