[Bioc-devel] issue about S4 slot has a dist object.
Michael Lawrence
lawrence.michael at gene.com
Thu Apr 2 22:19:12 CEST 2015
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 <michafla at 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 <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