[Bioc-devel] issue about S4 slot has a dist object.

[Michael Lawrence]

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.



On Wed, Apr 1, 2015 at 4:29 PM, Martin Morgan <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>
>>> wrote:
>>>
>>>  Hi,
>>>>
>>>> Here is the package. (https://tracker.bioconductor.org/issue1204 or
>>>> 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
>>>> >
>>>> 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>, 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>
>>>>
>>>>          [[alternative HTML version deleted]]
>>>>
>>>> _______________________________________________
>>>> Bioc-devel at r-project.org mailing list
>>>> https://stat.ethz.ch/mailman/listinfo/bioc-devel
>>>>
>>>>
>>>     [[alternative HTML version deleted]]
>>>
>>> _______________________________________________
>>> Bioc-devel at r-project.org mailing list
>>> 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
>

	[[alternative HTML version deleted]]