Bioconductor / BiocFileCache

Manage Files Across Sessions
12 stars 6 forks source link

Add makeCachedActiveBinding() #19

Closed hpages closed 4 years ago

hpages commented 4 years ago

This PR doesn't include reoxygenized man/BiocFileCache-class.Rd and man/makeBiocFileCacheFromDataFrame.Rd (too many changes, it looks like the package has not been reoxygenized after some significant changes to R/BiocFileCache-class.R and R/makeBiocFileCacheFromDataFrame.R).

hpages commented 4 years ago

Taking advantage of this, the .onLoad hook in org.Mxanthus.db will become:

loadOrgMxanthusDb <- function() {
    ah <- suppressMessages(AnnotationHub::AnnotationHub())
    ah[["AH75133", verbose=FALSE]]
}

.onLoad <- function(libname, pkgname)
{
    ns <- asNamespace(pkgname)
    makeCachedActiveBinding(pkgname, loadOrgMxanthusDb , env=ns)
    namespaceExport(ns, pkgname)
}

They'll also need to add

Imports: BiocFileCache (>= 1.11.1)

to their DESCRIPTION file and

importFrom(BiocFileCache, makeCachedActiveBinding)

to their NAMESPACE file.

hpages commented 4 years ago

And for PANTHER.db it will become:

make_PANTHER.db <- function() {
    ah <- suppressMessages(AnnotationHub())
    dbfile <- ah[["AH75194", verbose=FALSE]]
    ## Use AnnotationDbi::dbFileConnect() instead of DBI::dbConnect() to
    ## avoid some platform specific pitfalls.
    conn <- AnnotationDbi::dbFileConnect(dbfile)
    db <- new("PANTHER.db", conn=conn)
    db$.initializePANTHERdb()
    db
}

.onLoad <- function(libname, pkgname) {
    ns <- asNamespace(pkgname)
    makeCachedActiveBinding("PANTHER.db", make_PANTHER.db, env=ns)
    namespaceExport(ns, "PANTHER.db")
}

with the same additions to the DESCRIPTION and NAMESPACE files.

BTW I think doing

    makeCachedActiveBinding("PANTHER.db", make_PANTHER.db, env=ns)
    namespaceExport(ns, "PANTHER.db")

or

    sym <- "PANTHER.db"
    makeCachedActiveBinding(sym, make_PANTHER.db, env=ns)
    namespaceExport(ns, sym)

is more readable than doing

    makeCachedActiveBinding(pkgname, make_PANTHER.db, env=ns)
    namespaceExport(ns, pkgname)

The fact that the name of the active binding is the same as the package is pure coincidence so IMO not using pkgname for the name of the binding avoids some potential confusion.

lshep commented 4 years ago

Thanks @hpages Looking at this now - I'll make sure all the docs get reoxygenized/documented, not sure what changes to the docs I made that weren't updated but I'll look them all over. I'll reach out with the suggested code or do pull requests to the two annotation packages - The suggestion to use pkgname in the onLoad calls was from @mtmorgan as part of code simplicity and clean up but I'm fine with either -

lshep commented 4 years ago

I think the roxygen changes are using the new version of roxygen or simple formatting - There are no changes to the docs just the spacing and line wrapping - Merging and pushing shortly