Skip to content

[Bioc-devel] Why should Bioconductor developers re-use core classes?

17 messages · Levi Waldron, Dario Strbenac, Hervé Pagès +6 more

Original
Levi Waldron
# 
I'm putting together a presentation with a demo on why Bioconductor
developers should re-use and extend core classes whenever possible. It
includes a demo of some real-life consequences from two packages I use a
lot, metagenomeSeq and phyloseq. These are far from the only examples, many
Bioconductor packages have created new classes from scratch, and I think as
a community we should greatly reduce that practice. I would welcome any
feedback:

https://www.slideshare.net/LeviWaldron/why-reuse-core-classes

(sorry the slides are a little Frankenstein - in the interest of speed I
made part of it in Powerpoint and part in Beamer, and used pdftk to
concatenate these! In practice I would do the Beamer part as a live-demo,
and take advantage of some animations in PPT)
Dario Strbenac
# 
Good day,

I developed ClassifyR, which is a classification framework, based on ExpressionSet. Now that we're getting enquiries about inputting multiple datasets derived from the same patients, we plan to completely refactor the software to use MultiAssayExperiment as a foundation class.

--------------------------------------
Dario Strbenac
University of Sydney
Camperdown NSW 2050
Australia
Vincent Carey
# 
I found it a very congenial presentation.

One related issue -- perhaps -- a new HDF5 interface package, that is
based on R6!

https://github.com/hhoeflin/hdf5r

Core class consciousness is surely worthy of promotion.  But what about OOP
methodology?  I personally am happy with S4.  I believe there are some
packages
that use setRefClass ... any guidance on this?

On Tue, Oct 17, 2017 at 7:54 PM, Levi Waldron <lwaldron.research at gmail.com>
wrote:

  
  


  


      

    

    

      
      

        


  

        

      Michael Lawrence
    

    

      
      #
    

  


  

      
If Biocondutor integration is important, then reference classes
(setRefClass) are preferable, since they fully integrate with the rest of
S4, including class hierarchies and method dispatch. It's important not to
be confused by the R6 branding. It's (sort of) an alternative to S4, not an
evolution of it.

Does R now have more HDF5 interfaces or more OOP frameworks?

Michael

On Tue, Oct 17, 2017 at 5:16 PM, Vincent Carey <stvjc at channing.harvard.edu>
wrote:

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

        

  
  


  


      

    

    

      
      

        


  

        

      Hervé Pagès
    

    

      
      #
    

  


  

      
It should also be pointed out that reference classes classes are rarely
needed and can easily be used for the wrong reasons (e.g. performance?).
The pass-by-reference semantic they provide can fire back. Most of the
time objects don't need and should not have pass-by-reference semantic,
only *some* of their slots.

Nice slides Levi!

H.

        
On 10/17/2017 10:04 PM, Michael Lawrence wrote:

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

        

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Ryan
    

    

      
      #
    

  


  

      
I think the main reason for reusing/subclassing core classes that users can
appreciate is that it makes it much easier for users to integrate multiple
packages into a single workflow. Only the most basic of pipelines uses just
a single Bioconductor package. For instance, an "edgeR" pipeline obviously
uses the edgeR package, but it likely also uses several other packages,
like sva, RUV, variancePartition, etc. The more these different packages
operate on the same core data structures, the less work the user has to do
to use them together. And to bring that back around to an incentive for
developers, making your package interoperate with other packages more
easily means that users will be more likely to use your package.

On Tue, Oct 17, 2017 at 4:55 PM Levi Waldron <lwaldron.research at gmail.com>
wrote:

            

      
      
      
    

        

      
      
    

        

  
  


  


      

    

    

      
      

        


  

        

      Michael Lawrence
    

    

      
      #
    

  


  

      
Good points. I think Levi hit on the direct reuse argument (via inheritance
and composition), and, you're right, interoperability is another big one.

On Wed, Oct 18, 2017 at 10:26 AM, Ryan Thompson <rct at thompsonclan.org>
wrote:

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

        

  
  


  


      

    

    

      
      

        


  

        

      Levi Waldron
    

    

      
      #
    

  


  

          

      
      
      
    

        
My impression is that the interoperability argument may already be more
widely appreciated, because in the pipeline example you can have several
packages operating on the same data class. It seems less obvious when you
are doing something different that requires defining a new class, why you
should extend an existing class to meet your needs. Although I guess your
point extends to interoperability with other packages providing methods for
the parent class, and the ability to use coercion methods defined for the
parent class, which I didn't mention...

  
  


  


      

    

    

      
      

        


  

        

      Dario Strbenac
    

    

      
      #
    

  


  

      
Good day,

It might be useful to readers to have a comparison table (ticks and crosses) in the MultiAssayExperiment vignette that compares the features available in it to those available in SummarizedExperiment, to allow quicker decision making.

--------------------------------------
Dario Strbenac
University of Sydney
Camperdown NSW 2050
Australia

  
  


  


      

    

    

      
      

        


  

        

      Paul Joseph McMurdie
    

    

      
      #
    

  


  

      
Thanks, Levi, nice slides.

In case it is a helpful perspective, I'll try to share what I recall of my
thought process as author of phyloseq. And I should preface by admitting
that I've been embarrassed by this major development oversight for some
years now.

At the beginning of 2011 I was a new postdoc, heavy R user, completely new
to R development, and in a "cousin" field (microbiome & bacterial genomics)
that had virtually no presence in BioC. Some of the recommendations you've
made in this slide deck were not available then, but admittedly, I might
have missed them even if they were. I had access to training in base R
devel (Hadley's bootcamp, John Chambers' course at Stanford), but a lot of
resources in BioC were still very new to me.

If you can believe it, my original idea for the phyloseq-class was even
worse! Valerie Obenchain was great and patiently talked me into a better
solution, but somehow she also missed that I might have re-used available
classes and avoided some unnecessary implementation and maintenance.

What additional recommendations can I make with the benefit of hindsight
that have not been mentioned in this thread?

- Hopefully it is obvious from my description, and also what I imagine to
be Levi's motivation for making the slide deck, but somehow new eager
developers are missing out on this great infrastructure and it isn't
because they want to re-implement core stuff. I sure didn't! I simply
didn't know what was there or best practices for BioC. *A "beginner's guide
to BioC package development"* would have been at the top of my list of
things to read back then.

- It isn't that I didn't read other established packages. I did. However, a
lot of core BioC tools had gene-expression specific names even for data
classes that were not intrinsically gene expression (e.g. it's actually a
matrix, or related tables) -- and I'm happy many of these now use more
general names like "experiment" or "row". The old names signaled to me
"this isn't for you". And I naively, ignorantly, accepted that at mostly
face value.

- Conversely, sometimes not-inheriting methods is a feature, because it
protects users from doing something that is great and appropriate for one
domain (gene expression) but totally irrational in another (microbiome).
I'm not saying my original implementation made great nuanced decisions
about this -- it has many trappings of a new developer -- but I did have
some pretty naive users in mind with phyloseq, for whom navigating legacy
methods and method names from other domain(s) was expected to be a hurdle.
Curious to hear thoughts on this.

- There actually *still isn't core support for evolutionary trees in BioC* (as
mentioned by Joe Paulson and Ben Callahan in other threads). One of
phyloseq's key contributions was to leverage the fantastic representation
of trees implemented in the CRAN package "ape" in order to support analysis
techniques popular among microbiome researchers that require a phylogenetic
tree. The integration in the phyloseq-class and ape is necessarily pretty
deep, including certain row operations. Users also needed a familiar and
simple R interface to manipulate that composite object despite the complex
hierarchical relationship among rows. Correct me if I'm wrong, but I think
there is still no core BioC support for representing tree-like or
bio-taxonomy-like hierarchy among rows in a SummarizedExperiment, or
equivalent; and consequently certain row operations may have to be modified
more deeply than usual if we were to re-implement phyloseq "the right way".
I'd love to hear thoughts on this.

Even though phyloseq is at the receiving end, I think the criticism is
fair, and I want current and future new BioC contributors to not re-make my
mistakes circa 2011-12. I'm happy to help if I can.

Cheers, and thanks for the interesting, collegial thread.

Joey



---
---
"Joey"
Paul J. McMurdie II
Sent from Gmail


On Wed, Oct 18, 2017 at 11:46 AM, Levi Waldron <lwaldron.research at gmail.com>
wrote:

            

      
      
      
    

        

  
  


  


      

    

    

      
      

        


  

        

      Gmail
    

    

      
      #
    

  


  

      
I just converted my gene set data structures to GSEABase::GenSet
class. I think that one major advantage is that I just transferred the
burden of staying up to date with gene set formats to people working
specifically on that. If the file formats from the MSigDB change,
someone will work on updating the methods that import them to GeneSet
objects.

In general, there's someone in charge of a software module, and I'm
using both his original work, and his maintenance work.
P.S. the dark side: if they fail to maintain their code, yours will
fail too. I have been there.

Francesco


On Wed, Oct 18, 2017 at 1:54 AM, Levi Waldron

        
<lwaldron.research at gmail.com> wrote:

            

      
      
      
    

        

      
      
    

  
  


  


      

    

    

      
      

        


  

        

      Levi Waldron
    

    

      
      #
    

  


  

      
Thanks for all your thoughts Joey, and I hope I didn't come across as
critical of phyloseq in particular. In fact, the couple packages I created
as a post-doc (doppelgangR and ffpe) did exactly the same thing, but unlike
phyloseq have never been used enough for it to make much difference :).
This comparison with phyloseq and metagenomeSeq is just one that I have
personal experience with because I use both packages and it was something
of a revelation to me when I realized that MRExperiment and all other
eSet-derived classes would "just work" as elements of a
MultiAssayExperiment. My motivation in making these slides was mainly to
share my own very recent revelations and try to make life better for new
package developers and their users.  A couple comments below:


On Wed, Oct 18, 2017 at 11:36 PM, Paul Joseph McMurdie <joey711 at gmail.com>
wrote:

            

      
      
      
    

        
I think this is still not abundantly clear from
http://bioconductor.org/developers/ and I'm not sure how much it factors
into the package review process. Would it be helpful to have a short
questionnaire for creators of new packages that includes some things not
part of R CMD BiocCheck, or CHECK, like - what new classes do you define,
and what is the purpose of defining them?

            

      
      
      
    

        
Something you did well in phyloseq was define methods for all common user
operations, and if you had inherited from eSet, you probably still would've
wanted to do this - except that more of your methods would've been just
wrappers for inappropriately named eSet methods, and your average user
wouldn't have known the difference. Because of your use of methods and not
direct slot access by users, I think you could still change the underlying
data model without it being noticeable to basic users, even if it expanded
the number of methods actually available. Thankfully, SummarizedExperiment
I think now avoids these "this isn't for you" signals.

            

      
      
      
    

        
AFAIK you're right, and I don't know the solution, although I hope it can
be built on SummarizedExperiment. Looking forward to talking more about
this.

            

      
      
      
    

        
Thanks Joey, and I do want to say also that I think phyloseq is responsible
for making Bioconductor a viable and already superior choice for
statistical analysis of microbiome data!

  
  


  


      

    

    

      
      

        


  

        

      Vincent Carey
    

    

      
      #
    

  


  

      
On Thu, Oct 19, 2017 at 10:12 AM, Levi Waldron <lwaldron.research at gmail.com>
wrote:

            

      
      
      
    

        
Yes.  Let's write up the use cases and try to spec something out.  Maybe
start a topic on the support site, or a slack channel?  rowRanges seems
very useful, perhaps
a rowGraph/colGraph concept would be useful too.  Establishing idioms for
using
these in model specification would be nice.

            

      
      
      
    

        

      
      
    

        

  
  


  


      

    

    

      
      

        


  

        

      Michael Lawrence
    

    

      
      #
    

  


  

      
There may be a common pattern emerging where derivatives of
SummarizedExperiment add support for specific feature types. Currently, we
have ranges, and it was suggested in a package submission thread to support
gene sets (or pathways, etc). Those could also be represented by a graph,
or something more specific.

On Thu, Oct 19, 2017 at 8:06 AM, Vincent Carey <stvjc at channing.harvard.edu>
wrote:

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

  
  


  


      

    

    

      
      

        


  

        

      Martin Morgan
    

    

      
      #
    

  


  

      
On 10/19/2017 11:06 AM, Vincent Carey wrote:

            

      
      
      
    

        
I started a slack channel for this; all are welcome to participate but I 
don't know how to invite people

I believe people can join the Bioconductor community by visiting

   https://bioc-community.herokuapp.com/

Martin

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

        
This email message may contain legally privileged and/or...{{dropped:2}}

  
  


  


      

    

    

      
      

        


  

        

      Martin Morgan
    

    

      
      #
    

  


  

      
On 10/19/2017 10:12 AM, Levi Waldron wrote:

            

      
      
      
    

        
[ should have mentioned that the slack channel is #tree-like-se ]

A couple of quick comments on new packages / reviews.

New package submissions have a checklist that people submit with their 
package. Since it is not uncommon to find checked items that have not 
been satisfied, I'm quite cynical about this as a way of enforcing good 
behavior, though maybe it does something to elevate awareness.

One of the check-list items is acknowledge reading the package guidelines

   https://bioconductor.org/developers/package-guidelines/

which include an S4 section

   https://bioconductor.org/developers/package-guidelines/#classes

and links to general guidance

 
https://bioconductor.org/developers/how-to/efficient-code/#re-use-existing-functionality

as well as emphasis on core containers

   https://bioconductor.org/developers/how-to/commonMethodsAndClasses/

A relatively humorous pattern is for newer team members to eventually 
lament that new packages don't re-use existing classes and methods, 
suggesting that more documentation is the solution; usually the litany 
above is cited, and the newer team member either tweaks or adds to the 
existing documentation to the point that they think it is surely 
satisfactory, or sees that more documentation is not an effective 
answer. This pattern started again yesterday...

One thing that Joey said, and that I think is true of a lot of new 
package contributors, is that they are 'new' developers. This is 
somewhat ironic, since they are writing packages that are meant (and 
sometimes are, as with phyloseq) to be widely used and influential; it 
seems somehow appropriate to keep that channel open, with the domain and 
other expertise that 'new' developers bring as a trade-off against the 
sophistication of their current development skills. Of course being new 
developers mean that they make both 'mistakes' and choices that are not 
entirely consistent with the overall project, e.g., use of S3 or R6 
rather than S4 classes, reimplementation of existing functionality, 
adopting unfamiliar API conventions (a minor favorite of mine is the 
presentation of a novel method for some aspect of differential 
expression, with the primary input a matrix of samples x 'genes' rather 
than the bioinformatics convention of 'genes' x samples). The diversity 
of programming and analysis approaches available in R makes it not at 
all surprising that the insights that can come even after many years of 
working with Bioconductor are not accessible to many new package 
contributors.

I'll also mention my own philosophy (which I sometimes emphasize with 
other members of the core team of reviewers) in reviewing packages, 
which is to seek incremental gain rather than perfection; sometimes 
there are issues other than S4 class use (!) where greater strides can 
be made.

Martin

            

      
      
      
    

        

      
      
    

        
This email message may contain legally privileged and/or...{{dropped:2}}

  
  


  


      

    

    

      
      

        


  

        

      Paul Joseph McMurdie
    

    

      
      #
    

  


  

      
Thanks for those additional comments, Levi. I don't think you were being
unfair to phyloseq, and it sounds like some of the issues involved are
still relevant (e.g. new domain, new contributor).

I wanted to riff on Martin's comment about "incremental gain rather than
perfection". I imagine there are many reasons to use this as a guiding
principle, but one of them could be that it helps foster new-to-BioC
contributors by not overwhelming them with sub-critical requirements.
Interoperability is a feature that developers are already incentivized to
leverage, when they realize it, if they haven't buried themselves too deep
O:-). Similarly, after taking a peek at those developer docs, I think
buildingPackagesForBioC
<http://bioconductor.org/developers/how-to/buildingPackagesForBioc/> looks
pretty great, and I wish it had existed (or I had known about it) when I
unwittingly blundered these early decisions in phyloseq.

Thanks again for the discussion/feedback!



---
---
"Joey"
Paul J. McMurdie II
Sent from Gmail


On Thu, Oct 19, 2017 at 9:53 AM, Martin Morgan <

        
martin.morgan at roswellpark.org> wrote: