Skip to content

[Bioc-devel] Generic for write to BiocGenerics

8 messages · Rainer Johannes, Hervé Pagès, Michael Lawrence +1 more

Original
Rainer Johannes
# 
Dear all!

We are currently implementing mzML write support in `MSnbase` and did
implement a `write` method for the S4 objects in `MSnbase`. Now, the
question is whether it might not be better to define a `write` S4
generic in `BiocGenerics`?


cheers, jo
Hervé Pagès
# 
Hi jo,

At the moment probably not much to be gained unless you ran into
some conflicts with other "write" methods defined in Bioconductor?

Note that the arguments/signature of base::write() don't really
help making it a clean generic functions (e.g. weird 'file' default,
'ncolumns' and 'sep' args that lack generality, no ellipsis). It seems
to me that instead of trying to force write() into a generic, it might
be easier/cleaner to define a method for the export() generic defined
in rtracklayer. Maybe the discussion would be whether we should consider
moving rtracklayer::export() to BiocGenerics?

Finally, IMO there is nothing wrong in using specific write* names like
writeMgfData, writeMSData, writeMzTabData, etc... It plays nicely with
tab-completion, the user can use args() to quickly see all the args and
their defaults, tab-completion also works on the arguments, and the user
does not struggle in finding the man page (like s/he does sometimes
with generic and methods, especially when those are defined in
different packages).

H.
On 09/18/2017 10:21 AM, Rainer Johannes wrote:

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Michael Lawrence
    

    

      
      #
    

  


  

      
I was also about to suggest that we converge on export() but held back
because I was obviously biased. I also agree with making the target
explicit in the function name. It makes the intent of the code way
more obvious.

        
On Mon, Sep 18, 2017 at 3:15 PM, Herv? Pag?s <hpages at fredhutch.org> wrote:

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

  
  


  


      

    

    

      
      

        


  

        

      Rainer Johannes
    

    

      
      #
    

  


  

      
Thanks for the suggestions!

Yes, writeMSData was my first go - but we have already a writeMSData
function in mzR. I didn't want to make that a generic and implement it
for the MS data objects in MSnbase, because I thought that calling a
method `writeMSData,MSnExp` on an object the user knows to contain MS
data might be too much (i.e. it would translate to "write the MS data
from the MS data containing object to a file" while e.g. `write,MSnExp`
would translate to "write the MS data containing object to a file"). I
know is not the best reason for not using writeMSData -

Using export might be a good alternative - does also not interfere with
the base::write. Would be nice if you could add that to BiocGenerics.

thanks, jo

        
On 09/19/2017 12:22 AM, Michael Lawrence wrote:

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

  
  


  


      

    

    

      
      

        


  

        

      Laurent Gatto
    

    

      
      #
    

  


  

      
Out of curiosity, do you also have an import() function?

        
On 18 September 2017 22:22, Michael Lawrence wrote:

        

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

        

  
    

      
      
    

  


  


      

    

    
1 day later

    

      
      

        


  

        

      Hervé Pagès
    

    

      
      #
    

  


  

      
If we want to move export() to BiocGenerics of course import()
would need to follow.

My rough estimate is that this move would possibly break between
20-30 packages or more. The BioC 3.6 release is coming soon and
it's a bit late to introduce such a disruptive change. So I vote
for postponing this to BioC 3.7.

@jo: Would you consider making MSnbase depend on rtracklayer in
the mean time? Alternatively, my suggestion to use a specific
write* name still stands.

H.

        
On 09/18/2017 11:28 PM, Laurent Gatto wrote:

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Laurent Gatto
    

    

      
      #
    

  


  

      
On 21 September 2017 04:15, Herv? Pag?s wrote:

        

            

      
      
      
    

        
writeMSData is indeed the route we decided to go in the end, so no need
to move export() and import() for us at the moment.

Thanks again for your input.

Best wishes,

Laurent

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

      
      
    

            

      
      
      
    

        

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Hervé Pagès
    

    

      
      #
    

  


  

      
On 09/20/2017 09:49 PM, Laurent Gatto wrote:

            

      
      
      
    

        
Great to hear. Thanks Laurent for the update.

H.