nkurzaw commented 4 years ago

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

Repository: https://github.com/nkurzaw/Rtpca

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 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.

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 help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.

bioc-issue-bot commented 4 years ago

Hi @nkurzaw

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: Rtpca
Title: Thermal proximity co-aggregation with R
Version: 0.99.0
Authors@R: c(
    person("Nils", "Kurzawa", email = "nils.kurzawa@embl.de", 
        role = c("aut", "cre")),
    person("André", "Mateus", role = c("aut")),
    person("Mikhail M.", "Savitski", role = c("aut")))
Description: R package for performing thermal proximity co-aggregation 
    analysis with thermal proteome profiling datasets.
License: GPL-3
Encoding: UTF-8
VignetteBuilder: knitr
LazyData: false
biocViews: Software, Proteomics, DataImport
Depends: R (>= 4.0.0), stats, utils, Biobase, dplyr, tidyr
Imports: methods, ggplot2, pROC, fdrtool, splines
Suggests: knitr, BiocStyle, TPP, testthat
RoxygenNote: 7.1.0

bioc-issue-bot commented 4 years ago

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

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 4 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 build report for more details.

bioc-issue-bot commented 4 years ago

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

59150af addressed 'incomplete final line found' in two tes...

bioc-issue-bot commented 4 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 build report for more details.

bioc-issue-bot commented 4 years ago

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

21bf319 extended package description (BiocCheck), added NE...

bioc-issue-bot commented 4 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.

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

Please see the build report for more details.

Kayla-Morrell commented 4 years ago

Hello @nkurzaw,

Thank you for submitting to Bioconductor. Please see the initial review of the package below. The required changes must be made while the suggested changes do not have to be but we strongly encourage them. Comment back here with updates that have been made and when the package is ready for a re-review.

DESCRIPTION

[ ] REQUIRED: Depends are for packages that provide essential functionality for users of your package. It is unusual for more than three packages to be listed here. Please move some of these packages to imports.
[ ] SUGGESTION: It is encouraged to include a 'BugReports:' field with the relevant link to GitHub for reporting Issues.

Vignette

[ ] REQUIRED: The 'Installation' section should demonstrate to the user how to install the package from Bioconductor, not GitHub.
[ ] SUGGESTION: I would consider moving the subsections (2.1-2.3) under 'Introduction' into their own section since they aren't introductions but the actual demonstration of the functionality of the package.
[ ] SUGGESTION: I'm assuming the data 'hdacTR_smallExample' is coming from TPP? You might mention this.
[ ] SUGGESTION: It isn't clear (at least not to me) in the vignette when you move from TPP functionality into the functionality of Rtpca. I see under 2.1 where you mention importing using TPP, but then you don't mention if the other functions used are from there or Rtpca. Just a bit of clarification in the text might be helpful.
[ ] REQUIRED: What is the purpose of the .csl file? There should only be one reference file included here.

Man pages

[ ] REQUIRED: The only files that should be in this directory are the .Rd man pages. I would suggest moving the 'figures' directory to an 'inst/figure/' directory
[ ] REQUIRED: The class man page doesn't seem complete, the value information just ends at 3).

R code

[ ] REQUIRED: Avoid the use of direct slot access with @ or slot(). An accessor method should be created and utilized instead.
[ ] SUGGESTION: For formating reasons, consider shorter lines. There are 26 lines that are > 80 characters long.
[ ] SUGGESTION: For formating reasons, consider multiples of 4 spaces for line indents. There are 168 lines that are not

Best, Kayla

nkurzaw commented 4 years ago

Hello @Kayla-Morrell, thanks a lot for reviewing our package and for getting back to us so fast. I will try to address all the points you raised as soon as possible!

Concerning the .csl file in the vignette folder, it is a citation style that I prefer over the standard rmarkdown one, I have that included also for another Bioconductor package of mine (TPP2D) and it does not cause any troubles there. But if you think I should still remove it, I am happy to do that.

Thanks again! Best, Nils

bioc-issue-bot commented 4 years ago

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

0057fbe adressed first rounds of comments, still some left

bioc-issue-bot commented 4 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.

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

Please see the build report for more details.

Kayla-Morrell commented 4 years ago

@nkurzaw - That's fine to leave the .csl file then, thank you for clarifying!

nkurzaw commented 4 years ago

Ok great, thanks a lot!

bioc-issue-bot commented 4 years ago

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

4ab653f added more structure and explanations to vignette,...

bioc-issue-bot commented 4 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.

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

Please see the build report for more details.

Kayla-Morrell commented 4 years ago

Hello @nkurzaw - Just checking in to see if the package is ready for re-review? If it is, please comment back addressing the points I raised. If not, just let me know when it's ready. Best, Kayla

nkurzaw commented 4 years ago

Hello @Kayla-Morrell, I'm sorry, I'm not quite there yet. I'm still missing some accessor functions. I'll try to finalise all required changes and comment back on the points tomorrow!

Best, Nils

bioc-issue-bot commented 4 years ago

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

8822532 adressed remaining required review comments by add...

bioc-issue-bot commented 4 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.

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

Please see the build report for more details.

nkurzaw commented 4 years ago

Hi @Kayla-Morrell ,

I have now addressed all 'REQUIRED' change requests and most 'SUGGESTIONS'. Thanks a lot again for reviewing our package, you had great comments that definitely improved the package! Please see below the detailed response to all comments:

DESCRIPTION

[x] REQUIRED: Depends are for packages that provide essential functionality for users of your package. It is unusual for more than three packages to be listed here. Please move some of these packages to imports.
I have now reduced the Depends to: "R (>= 4.0.0), stats, dplyr, tidyr". All of the three packages are required for core functionalities of the package.
[x] SUGGESTION: It is encouraged to include a 'BugReports:' field with the relevant link to GitHub for reporting Issues.
the following 'BugReports' field has been added to the DESCRIPTION file: "BugReports: https://support.bioconductor.org/"

Vignette

[x] REQUIRED: The 'Installation' section should demonstrate to the user how to install the package from Bioconductor, not GitHub.
This has been changed accordingly. It now states:

" Installation from Bioconductor

if (!requireNamespace("BiocManager", quietly = TRUE))
    install.packages("BiocManager")
BiocManager::install("Rtpca")

"

[x] SUGGESTION: I would consider moving the subsections (2.1-2.3) under 'Introduction' into their own section since they aren't introductions but the actual demonstration of the functionality of the package.
The vignette has now been restructured: a new heading after "Installation" and "Introduction" called "The Rtpca package workflow" has beed added.
[x] SUGGESTION: I'm assuming the data 'hdacTR_smallExample' is coming from TPP? You might mention this.
You're right, this is now explained more clearly
[x] SUGGESTION: It isn't clear (at least not to me) in the vignette when you move from TPP functionality into the functionality of Rtpca. I see under 2.1 where you mention importing using TPP, but then you don't mention if the other functions used are from there or Rtpca. Just a bit of clarification in the text might be helpful.
Thanks, this is really a good point. This is now made more obvious by adding a subsection "Import Thermal proteome profiling data using the TPP package" and then another "Performing thermal co-aggregation analysis with Rtpca"
[x] REQUIRED: What is the purpose of the .csl file? There should only be one reference file included here.
This has been discussed already above

Man pages

[x] REQUIRED: The only files that should be in this directory are the .Rd man pages. I would suggest moving the 'figures' directory to an 'inst/figure/' directory
I changed this according to your suggestion
[x] REQUIRED: The class man page doesn't seem complete, the value information just ends at 3).
Good spot! This has now been completed

R code

[x] REQUIRED: Avoid the use of direct slot access with @ or slot(). An accessor method should be created and utilized instead.
I have now added accessor methods and replaced all usages of @ or slot
[x] SUGGESTION: For formating reasons, consider shorter lines. There are 26 lines that are > 80 characters long.
I have shortened all lines for which I was able to do so. The two remaining ones with > 80 characters are hyperlinks that cannot be broken.
[ ] SUGGESTION: For formating reasons, consider multiples of 4 spaces for line indents. There are 168 lines that are not
I have not completely addressed this point. I am using RStudio and it does not in all cases adhere to this style, if you feel strongly about this point, I'm happy to address it, but I don't think it adds much to the readability of the code.

Thank you very much for your time and effort spent again!

Best, Nils

Kayla-Morrell commented 4 years ago

@nkurzaw - Thank you for making the necessary changes! Everything looks good now and I'd be more than happy to accept the package.

Best, Kayla

bioc-issue-bot commented 4 years ago

Your package has been accepted. It will be added to the Bioconductor Git repository and nightly builds. Additional information will be posed to this issue in the next several days.

Thank you for contributing to Bioconductor!

nkurzaw commented 4 years ago

Awesome :) Thanks a lot @Kayla-Morrell!

Best, Nils

mtmorgan commented 4 years ago

The master branch of your GitHub repository has been added to Bioconductor's git repository.

To use the git.bioconductor.org repository, we need an 'ssh' key to associate with your github user name. If your GitHub account already has ssh public keys (https://github.com/nkurzaw.keys is not empty), then no further steps are required. Otherwise, do the following:

See further instructions at

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

for working with this repository. See especially

https://bioconductor.org/developers/how-to/git/new-package-workflow/ https://bioconductor.org/developers/how-to/git/sync-existing-repositories/

to keep your GitHub and Bioconductor repositories in sync.

Your package will be included in the next nigthly 'devel' build (check-out from git at about 6 pm Eastern; build completion around 2pm Eastern the next day) at

https://bioconductor.org/checkResults/

(Builds sometimes fail, so ensure that the date stamps on the main landing page are consistent with the addition of your package). Once the package builds successfully, you package will be available for download in the 'Devel' version of Bioconductor using BiocManager::install("Rtpca"). The package 'landing page' will be created at

https://bioconductor.org/packages/Rtpca

If you have any questions, please contact the bioc-devel mailing list (https://stat.ethz.ch/mailman/listinfo/bioc-devel); this issue will not be monitored further.

Bioconductor / Contributions

Rtpca #1508

DESCRIPTION

Vignette

Man pages

R code

DESCRIPTION

Vignette

Man pages

R code