Contributions icon indicating copy to clipboard operation
Contributions copied to clipboard

protlocassign

Open mooredf22 opened this issue 3 years ago • 52 comments
trafficstars

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

  • Repository: https://github.com/mooredf22/protlocassign

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

  • [x] I understand that by submitting my package to Bioconductor, the package source and all review commentary are visible to the general public.

  • [x] I have read the Bioconductor Package Submission instructions. My package is consistent with the Bioconductor Package Guidelines.

  • [x] I understand Bioconductor Package Naming Policy and acknowledge Bioconductor may retain use of package name.

  • [x] I understand that a minimum requirement for package acceptance is to pass R CMD check and R CMD BiocCheck with no ERROR or WARNINGS. Passing these checks does not result in automatic acceptance. The package will then undergo a formal review and recommendations for acceptance regarding other Bioconductor standards will be addressed.

  • [x] My package addresses statistical or bioinformatic issues related to the analysis and comprehension of high throughput genomic data.

  • [x] I am committed to the long-term maintenance of my package. This includes monitoring the support site for issues that users may have, subscribing to the bioc-devel mailing list to stay aware of developments in the Bioconductor community, responding promptly to requests for updates from the Core team in response to changes in R or underlying software.

  • [x] I am familiar with the Bioconductor code of conduct and agree to abide by it.

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

  • [x] The 'devel' branch for new packages and features.
  • [x] The stable 'release' branch, made available every six months, for bug fixes.
  • [x] Bioconductor version control using Git (optionally via GitHub).

For questions/help about the submission process, including questions about the output of the automatic reports generated by the SPB (Single Package Builder), please use the #package-submission channel of our Community Slack. Follow the link on the home page of the Bioconductor website to sign up.

Comments: The "protlocassign" package presents R functions that take use mass spectrometry data from serial fractionation experiments to proportionally assign proteins to possibly multiple residences in subcellular organelles. The package includes seven vignettes that describe the methods in detail. There is also an accompanying manuscript describing the methods; a revision is under review.

Here are my comments on BiocCheck notes:

1 "Consider adding these automatically suggested biocViews: RNASeq"
This is not relevant to this proteomics-related package

2 "Recommended function length <= 50 lines."
Many of the functions that are longer than 50 lines create complex plots. There is no natural way to break them up into smaller components.

3 "Consider adding runnable examples to the following man pages which document exported objects:"
Most of the functions that are not service functions include runnable examples. The small number that are not are explained and illustrated in detail in the accompanying vignettes; setting up runnable examples for these functions withing the help files would be cumbersome.

4 "Consider shorter lines; 235 lines (3%) are > 80 characters long."
All lines in the functions are shorter than 80 characters. Much of the explanatory text in the vignettes are longer than 80 characters, and we believe that that text is readable and clear in its current form.

5 "Consider multiples of 4 spaces for line indents, 1133 lines(15%) are not."
We have chosen to use descriptive variable names in our code in order to clearly link them to the to the concepts explained in the vignettes and in the accompanying manuscript. As a consequence, some of these variable names are rather long, which makes it difficult to adhere strictly to the "multiples of 4 spaces" suggestion. We have chosen indentation amounts to be as readable as possible and also conform to the limit of 80 characters per line.

6 "Cannot determine whether maintainer is subscribed to the bioc-devel mailing list (requires admin credentials).\nSubscribe here: https://stat.ethz.ch/mailman/listinfo/bioc-devel" I am listed as the maintainer and I have subscribed to this mailing list.

mooredf22 avatar Feb 27 '22 02:02 mooredf22

Hi @mooredf22

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: protlocassign
Title: Compute constrained proportional assignments for 
    fractionated protein abundance 
Version: 0.99.0
Date: 2022-02-26
Authors@R: 
    person(given = "Peter",
 family = "Lobel", 
 role = c("aut"),
 email = "[email protected]")
    person(given = "Dirk",
 family = "Moore",
 role = c("cre","aut"),
 email = "[email protected]")
Description: This package fits a constrained proportional assignment (cpa)
   model to mass spectrometry data 
    to proportionally assign proteins to subcellular locations.
    The package includes transformations to 
    improve the accuracy of the assignments as well as
    facilities for simulating mixtures and evaluating 
    the perfomance of cpa on those mixtures.
    Also included are facilities for summarizing spectral-level 
    data using random effects models to calculate average protein profiles.
License: GPL-3
Encoding: UTF-8
LazyData: false
Depends: 
    R (>= 4.1.0),
    lme4
RoxygenNote: 7.1.2
Imports:
    BB,
    pracma,
    outliers,
    knitr,
    stats,
    graphics,
    viridisLite,
    viridis,
    plot.matrix,
    grid,
    gridExtra,
    rmarkdown,
    utils,
    methods,
    BiocParallel
biocViews: Proteomics, MassSpectrometry
VignetteBuilder: knitr
URL: http//github.com/mooredf22/protlocassign
Suggests: 
    testthat (>= 3.0.0)
Config/testthat/edition: 3

bioc-issue-bot avatar Feb 27 '22 02:02 bioc-issue-bot

Hi -- this very comprehensively documented package will have to change in certain respects to be part of Bioconductor. I find that the built package is 13MB in size. I'll look more closely at that in a minute. The first vignette has

Start by installing the devtools package from CRAN, by typing:

install.packages("devtools")
Then install the protlocassign package from the github repository by typing:

devtools::install_github("mooredf22/protlocassign")
This will make the programs and data sets available. Also, several libraries are required which may be downloaded from CRAN by typing:

install.packages(c("BB", "pracma", "lme4", "outliers"))

and this is not appropriate. BiocManager::install will be used. See the developer guidelines.

We have some large html files in the built package:

-rw-r--r--  0 vincentcarey staff 5828512 Feb 26 20:53 protlocassign/inst/doc/vignette_4_more_on_mixtures.html
-rw-r--r--  0 vincentcarey staff   15016 Feb 26 20:55 protlocassign/inst/doc/vignette_5_Using_classical_fractionation_with_five_compartments_on_CPA.R
-rw-r--r--  0 vincentcarey staff   20066 Feb 26 20:48 protlocassign/inst/doc/vignette_5_Using_classical_fractionation_with_five_compartments_on_CPA.Rmd
-rw-r--r--  0 vincentcarey staff 6915101 Feb 26 20:55 protlocassign/inst/doc/vignette_5_Using_classical_fractionation_wi

It isn't completely clear to me why this is -- perhaps your graphics format is not an efficient one.

Finally, I think the computations for the vignettes are going to take too long for the build system. Have you timed the build/ check process? You probably want to distinguish between software documentation in the vignette and an analysis workflow that uses your package, and create a workflow package that can have a longer build time.

vjcitn avatar Feb 27 '22 03:02 vjcitn

I have uploaded a revised version of protlocassign. Here are the changes:

  1. We have updated all discussions in the vignettes of installing packages to using the Bioconductor version of install.
  2. We have created small subsets of our datasets for use in the vignettes. This has reduced the build time a great deal. On my Windows PC the build time (including the vignettes) was 333 seconds, and the check time (including vignettes) was 716 seconds.
  3. We have deleted all plots from the appendices of Vignettes 3 and 4, reducing their size. Below is a screenshot of the sizes of the vignettes: protlocassignVignetteSize2Mar2022

All but one of them are below 1 megabyte, and vignette 5 is only slightly above 1 megabyte.

If these are still too large, we can consider deleting some of them and link to (perhaps) a new github site containing the full set of vignettes. We believe, however, that this would be an inconvenience for users of the package.

mooredf22 avatar Mar 02 '22 17:03 mooredf22

Package builds to 8MB which is too big. But it can be reviewed.

vjcitn avatar Mar 09 '22 12:03 vjcitn

A reviewer has been assigned to your package. Learn what to expect during the review process.

IMPORTANT: Please read this documentation for setting up remotes to push to git.bioconductor.org. It is required to push a version bump to git.bioconductor.org to trigger a new build.

Bioconductor utilized your github ssh-keys for git.bioconductor.org access. To manage keys and future access you may want to active your Bioconductor Git Credentials Account

bioc-issue-bot avatar Mar 09 '22 13:03 bioc-issue-bot

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, TIMEOUT, skipped". 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 build report for more details. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to [email protected]:packages/protlocassign to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Mar 09 '22 13:03 bioc-issue-bot

@mooredf22 Please correct ERROR and TIMEOUT before a more indepth review will continue.

lshep avatar Mar 25 '22 12:03 lshep

This issue is being closed because there has been no progress for an extended period of time. You may reopen the issue when you have the time to actively participate in the review / submission process. Please also keep in mind that a package accepted to Bioconductor requires a commitment on your part to ongoing maintenance.

Thank you for your interest in Bioconductor.

bioc-issue-bot avatar Apr 05 '22 12:04 bioc-issue-bot

There has been no response and no effort to correct the current build ERROR's. If you plan on proceeding with the submission please request that this issue be reopened but only when you are ready to actively address package issues.

lshep avatar Apr 05 '22 12:04 lshep

I request that this submission, https://github.com/Bioconductor/Contributions/issues/2541, be re-opened. We have re-worked this package extensively to make it suitable for Bioconductor. In particular, while previously we had seven vignettes, we have now combined them into a single file, and refer to them as tutorials. We have also eliminated many figures. In this way, we have reduced the package size considerably. On my Windows machine, the package binary is 2.34 MB and the vignette (tutorials) is 2.54 MB. We have considerably re-worked the help files and added unit tests. This also has reduced the build time.

The vignette (tutorials) corresponds to those we are using as supplemental material for a manuscript currently under review. We hope that the current size is satisfactory so that it will correspond closely with the version in our manuscript.

mooredf22 avatar Apr 12 '22 20:04 mooredf22

I would like to reopen the protlocassign submission. It had taken us some time to re-work this package to make it suitable for Bioconductor. We have endeavored to answer all of the issues with this submission, particularly reducing its size and build time.

This is my first submission to Bioconductor, and I am still learning how to follow the policy on how to reopen this case. I have uploaded the revised package to github and increased the version number. I have also entered a new comment on the (currently inactive) submission site.

Thank you for your consideration

Sent from my iPhone

On Apr 5, 2022, at 8:05 AM, lshep @.***> wrote:

 There has been no response and no effort to correct the current build ERROR's. If you plan on proceeding with the submission please request that this issue be reopened but only when you are ready to actively address package issues.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

mooredf22 avatar Apr 12 '22 21:04 mooredf22

Dear @mooredf22 ,

We have reopened the issue to continue the review process. Please remember to push a version bump to git.bioconductor.org to trigger a new build.

bioc-issue-bot avatar Apr 14 '22 11:04 bioc-issue-bot

@mooredf22 - just checking weather you have pushed your latest changes to the Bioconductor git server, as the last commit I see is

Author: Moore <[email protected]>
Date:   Wed Mar 2 12:27:20 2022 -0500

    Use small test data sets for vignettes to reduce build times.

and you mentioned recent changes in your previous message.

lgatto avatar Apr 14 '22 12:04 lgatto

Before the submission was closed, the version was 0.99.1 Now it is 0.99.2

If I need to increment it again, let me know. Thanks!

Sent from my iPhone

On Apr 14, 2022, at 8:20 AM, Laurent Gatto @.***> wrote:

 @mooredf22 - just checking weather you have pushed your latest changes to the Bioconductor git server, as the last commit I see is

Author: Moore @.***> Date: Wed Mar 2 12:27:20 2022 -0500

Use small test data sets for vignettes to reduce build times.

and you mentioned recent changes in your previous message.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

mooredf22 avatar Apr 14 '22 12:04 mooredf22

You need to push to git.bioconductor.org. Please activate your git credentials account if you did not do so already and you can manage your ssh keys for access there. Please then see the new package workflow for how to set up remotes and push to git.bioconductor.org

lshep avatar Apr 14 '22 12:04 lshep

@mooredf22 do you need any help to push your latest changes to the Bioconductor git server?

lgatto avatar Apr 15 '22 19:04 lgatto

Received a valid push on git.bioconductor.org; starting a build for commit id: 8e7534f8f8816ff9fd3a8b917600b988749b12c1

bioc-issue-bot avatar Apr 15 '22 20:04 bioc-issue-bot

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, WARNINGS, TIMEOUT". 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 build report for more details. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to [email protected]:packages/protlocassign to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Apr 15 '22 20:04 bioc-issue-bot

I am revising the vignette code to decrease the build time and size.

mooredf22 avatar Apr 20 '22 21:04 mooredf22

Thank you - I have started the review anyway but didn't mange to finish it yet.

lgatto avatar Apr 21 '22 13:04 lgatto

Received a valid push on git.bioconductor.org; starting a build for commit id: 1503e07dc14898a79fb4f20ce001c96606266d3d

bioc-issue-bot avatar Apr 26 '22 01:04 bioc-issue-bot

I have added two new data frames to the package that contain pre-computed values for figures with panels of tables in Tutorials 4 and 5. I also modified the "mixtureHeatMap" function to optionally use these pre-computed values in place of computing them during the knitting of the vignette (tutorials) during the package build. I also used "\dontrun{}" in the help file for the "mixtureHeatMap" function. These two changes have significantly reduced the build time for the package without impacting the contents of the vignette (i.e. tutorials).

mooredf22 avatar Apr 26 '22 01:04 mooredf22

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 build report for more details. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to [email protected]:packages/protlocassign to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Apr 26 '22 01:04 bioc-issue-bot

The main concern with the build report appears to be the size of the vignette (which is a compendium of seven tutorials). These tutorials are being published as supplementary material for a companion manuscript that has recently been accepted for publication in the Journal of Proteome Research. I understand that there are resource limitations, but I (and my co-authors) would like to find a way to make this vignette acceptable for Bioconductor while deviating as little as possible from the published version.

mooredf22 avatar Apr 26 '22 02:04 mooredf22

Would it make sense than to keep a smaller, proof of principle vignette inside the package with the R code and publish an accompanied workflow package(s) that are more encompassing workflow packages with the full tutorials

lshep avatar Apr 27 '22 12:04 lshep

Yes, thanks, we will do this. We will submit both the main package (protlocassign) and the "workflow" (documentation) package (protlocassigndoc) at the same time when they are ready.

mooredf22 avatar Apr 27 '22 21:04 mooredf22

Received a valid push on git.bioconductor.org; starting a build for commit id: 1cf6af3d7dde0844c546fca444b33804b406df2c

bioc-issue-bot avatar Apr 29 '22 15:04 bioc-issue-bot

Dear Package contributor,

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

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

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to [email protected]:packages/protlocassign to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Apr 29 '22 15:04 bioc-issue-bot

The companion paper to the protlocassign package has just appeared in the Journal of Proteome Research:

A Method to Estimate the Distribution of Proteins across Multiple Compartments Using Data from Quantitative Proteomics Subcellular Fractionation Experiments Dirk F. Moore*, David E. Sleat, and Peter Lobel*

https://pubs.acs.org/doi/abs/10.1021/acs.jproteome.1c00781

mooredf22 avatar May 10 '22 01:05 mooredf22

Received a valid push on git.bioconductor.org; starting a build for commit id: 3ad17c2b4cf1a1609e4475941c7ac3979a9be63c

bioc-issue-bot avatar Jun 04 '22 01:06 bioc-issue-bot