search

Bioconductor / Contributions

Contribute Packages to Bioconductor
135 stars 33 forks source link

BiodivoTools package submission #494

Closed MPiet11 closed 7 years ago

MPiet11 commented 7 years ago

Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor

Confirm the following by editing each check box to '[x]'

I am familiar with the essential aspects of Bioconductor software management, including:

For help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.

bioc-issue-bot commented 7 years ago

Hi @MPiet11

Thanks for submitting your package. We are taking a quick look at it and you will hear back from us soon.

The DESCRIPTION file for this package is:

Package: BiodivoTools
Version: 0.99.0
Type: Package
Date: 2017-09-24
Title: Tools for Analysis of Diversity and Similarity in Biological
        Systems
Author: Christoph Sadee, Maciej Pietrzak, Michal Seweryn, Grzegorz Rempala
Maintainer: Maciej Pietrzak <pietrzak.20@osu.edu>
Depends: R (>= 3.3.0), cluster
Description: A set of tools for empirical analysis of diversity (a number and frequency of different types in population) and similarity (a number and frequency of shared types in two populations) in biological or ecological systems. 
Imports: grDevices, graphics, stats, utils
License: GPL (>=3)
NeedsCompilation: yes
biocViews: Microbiome, GeneExpression, Sequencing, Microarray,
        Clustering, Coverage
Packaged:
bioc-issue-bot commented 7 years ago

Your package has been approved for building. Your package is now submitted to our queue.

IMPORTANT: Please read the instructions for setting up a push hook on your repository, or further changes to your repository will NOT trigger a new build.

bioc-issue-bot commented 7 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "ERROR, WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/BiodivoTools_buildreport_20170925051416.html

bioc-issue-bot commented 7 years ago

Received a valid push; starting a build. Commits are:

5d9d59a Update DESCRIPTION

bioc-issue-bot commented 7 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/BiodivoTools_buildreport_20170925092812.html

MPiet11 commented 7 years ago

Hello Vobencha, I have a question regarding warnings produced by biocCheck: What would you suggest to put into \value sections of the man pages? Also I am not sure how to register native routines. Could you please suggest some reading for non-programmers? Is there anything else I should change/add in order to meet Bioconductor submission criteria? Greetings, Maciej

vobencha commented 7 years ago

Hi Maciej, Your package needs to meet the criteria in the new package guidelines: http://www.bioconductor.org/developers/package-guidelines/

This R manual talks about /value sections and registering native routines: https://cran.r-project.org/doc/manuals/r-release/R-exts.html

Valerie

MPiet11 commented 7 years ago

Hi Valerie, Thank you for clarification. Actually I used this references to prepare the package. I will edit man files, but the registration of native routines might be more challenging (I am still not sure how it will affect the performance of the functions). Is the registration mandatory for the acceptance? Best, Maciej

bioc-issue-bot commented 7 years ago

Received a valid push; starting a build. Commits are:

fd1ff7e Update DESCRIPTION

bioc-issue-bot commented 7 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/BiodivoTools_buildreport_20170926132741.html

MPiet11 commented 7 years ago

Dear Valerie, We are experiencing problems with registering native routines (when we follow both CRAN instructions and other resources available on the web). Since lack of registration of native routines does not affect functionality or speed of the package, would it be OK to postpone it and address this issue in the next version of the package?

Best, Maciej

vobencha commented 7 years ago

What problems are you having? Can you show the error/output?

If you need an example, you could look at how the source code is registered in Rsamtools, IRanges, GenomicAlignments or Biostrings packages. Seeing it implemented in a package might be more helpful than just the instructions.

vobencha commented 7 years ago

FYI, deadline for packages to get in BioC 3.6 is next Tuesday, Oct 17: http://www.bioconductor.org/developers/release-schedule/

bioc-issue-bot commented 7 years ago

Received a valid push; starting a build. Commits are:

c381756 Update DESCRIPTION

bioc-issue-bot commented 7 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the following build report for more details:

http://bioconductor.org/spb_reports/BiodivoTools_buildreport_20171015213337.html

MPiet11 commented 7 years ago

Hi Valerie, I am unable to replicate an error that occurred in tokay1 BUILD SRC output:

Error in find_vignette_product(name, by = "texi2pdf", engine = engine) : Failed to locate the 'texi2pdf' output file (by engine 'utils::Sweave') for vignette with name 'BiodivoTools-Manual'. The following files exist in directory '.': 'BiodivoTools-Manual-concordance.tex', 'BiodivoTools-Manual.Rnw', 'BiodivoTools-Manual.tex', 'BiodivoTools-Vignette.Rnw' Calls: -> find_vignette_product

Could you please provide more details. Best, Maciej

vobencha commented 7 years ago

This is a transient error we sometimes see with building the vignette. This is not related to registering native routines. You can ignore this error for now in the interest of going forward with the package review.

I'll do a full review of the package tomorrow.

vobencha commented 7 years ago

Hi,

Please see comments / suggestions below.

1) Bioconductor infrastructure

The package does not (re)use any Bioconductor core infrastructure, e.g., GenomicRanges, IRanges, SummarizedExperiment etc. It's not clear to me what type of data these analysis can be run on. If it's RNA seq I would suggest you (i) put the sample data in a SummarizedExperiment or RangedSummarizedExperiment object and (ii) modify the functions such that they support a SummarizedExperiment object as input and potentially as the output.

Please clarify what type of data this packages is intended for - this should be clear in the Description field of DESCRIPTION and in the vignette. Once that's clear we can work on integrating the package with the Biocondcutor infrastructure. There isn't much point in submitting to the Biocondcutor repo of you don't leverage the existing infrastructure. The idea is that users can have their data in a common set of classes that are supported across many packages. This makes moving the data around and applying functions from different packages more convenient.

2) function names

Your function names are not descriptive of what they do, e.g., what does a function called 'x' do? Please rename the functions to something that reflects their purpose.

~/BiodivoTools >ls man
BiodivoTools.Rd  dp.ht.Rd  ens.ht.Rd  i.inp.Rd  ji.Rd  mh.Rd     pg.Rd  srd.Rd       x.Rd
cvg.Rd           dp.Rd     ens.Rd     i.in.Rd   li.Rd  pg.ht.Rd  rd.Rd  TCR.Data.Rd

3) data set man page

This man page should describe, what the data are, the R object, dimension, names and what QC was done to get the data in the current form.

> data(TCR.Data)
> objects()
[1] "x"
> class(x)
[1] "matrix"
> dim(x)
[1] 6208    8
> dimnames(x)
[[1]]
NULL

[[2]]
[1] "Col.Naive"  "Col.Treg"   "PLN.Naive"  "PLN.Treg"   "MLN.Naive"
[6] "MLN.Treg"   "Thym.Naive" "Thym.Treg"

4) function man pages

All man pages need a \description, \usage, \arguments, \value section.

All man pages need more in-depth examples.

For the man pages that do have a \value section, they are incomplete. For example, i.inp(). Is the result always a list of length 3 with these names?

> result <- i.inp(x, resample = 100)
> names(result)
[1] "Mean"           "Lower.Quantile" "Upper.Quantile"

For an example of a complete man page, you could look at one from a Bioconductor core package, e.g.,

library(GenomicAlignments)
?findCompatibleOverlaps
?sequenceLayer

5) vignette

I'm not sure why you have the 2 files here. -Vignette is very short and not informative. -Manual is more complete but the code chunks are not evaluated. Can you combine the two? If they are combined and evaluated they could be a complete vignette.

The *-Vignette file has this line:

"Below is a short example of how to use the package. The full BiodivoTools User's Guide is available as part of the online documentation. "

followed by an extremely short example and then this line:

"For more information see BiodivoTools help and package manual. "

Are you referring to the *-Manual doc or some other documentation on a website somewhere?

Valerie

MPiet11 commented 7 years ago

Hi Valerie,

I completely agree with your suggestions. In current form the package is "just" a set of functions that can be used in many different contexts. Due to its universal nature it might be less clear where it fits in Bioconductor environment. Currently we are in the process of designing a wrapper package, that will include pipelines and examples for particular type of analysis. That one will have more integration with Bioconductor; will us Bioconductor data structures for input/output and will contain a manual with ready to 'copy/paste' examples. I think that this package would be more align with Bioconductor mission. Therefore we will suspend current submission and will submit more "bio-oriented" package. Please advice who should we contact (on the top of the forum) to dicuss Bioconductor-specific features of the package.

Thank you again for your feedback.

Best, Maciej

vobencha commented 7 years ago

OK, thanks for the explanation. I'm going to close this issue. When you're ready to submit the new version, request that we re-open this issue if the package has the same name or open a new issue if you've renamed it.

For help integrating Bioconductor-specific features, there are quite a few resources on this page https://bioconductor.org/developers/

Or you could write the bioc-devel mailing list with specific questions: https://stat.ethz.ch/mailman/listinfo/bioc-devel

Valerie