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

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]]

--
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