Bioconductor / Contributions

Contribute Packages to Bioconductor
131 stars 33 forks source link

PIPETS #3166

Closed qfurumo closed 4 months ago

qfurumo commented 9 months 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 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.

bioc-issue-bot commented 9 months ago

Hi @qfurumo

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: PIPETS
Title: Poisson Identification of PEaks from Term-Seq data
Version: 0.99.0
Authors@R: 
    person("Quinlan", "Furumo", email="furumo@bc.edu", role = c("aut", "cre"), comment = c(ORCID = "0000-0003-4486-302X"))
Description: PIPETS provides statistically robust analysis for 3'-seq/term-seq data. It utilizes a sliding window approach to apply a Poisson Distribution test to identify genomic positions with termination read coverage that is significantly higher than the surrounding signal. PIPETS then condenses proximal signal and produces strand specific results that contain all significant termination peaks. 
License: GPL-3
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.2.3
Imports: dplyr, utils, stats
Suggests: 
    knitr,
    rmarkdown,
    testthat (>= 3.0.0)
VignetteBuilder: knitr
URL: https://github.com/qfurumo/PIPETS
biocViews: Sequencing, Transcription, GeneRegulation, PeakDetection, Genetics, Transcriptomics, Coverage
Config/testthat/edition: 3
bioc-issue-bot commented 9 months ago

Your package has been added to git.bioconductor.org to continue the pre-review process. A build report will be posted shortly. Please fix any ERROR and WARNING in the build report before a reviewer is assigned or provide a justification on why you feel the ERROR or WARNING should be granted an exception.

IMPORTANT: Please read this documentation for setting up remotes to push to git.bioconductor.org. All changes should be pushed to git.bioconductor.org moving forward. It is required to push a version bump to git.bioconductor.org to trigger a new build report.

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 commented 9 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Build System.

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

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): PIPETS_0.99.0.tar.gz macOS 12.6.5 Monterey: PIPETS_0.99.0.tar.gz

Links above active for 21 days.

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

bioc-issue-bot commented 9 months ago

A reviewer has been assigned to your package for an indepth review. Please respond accordingly to any further comments from the reviewer.

jianhong commented 8 months ago

Package 'PIPETS' Review

Thank you for submitting your package to Bioconductor. The package passed check and build. However there are several things need to be fixed. Please try to answer the comments line by line when you are ready for a second review. Code: Note: please consider; Important: must be addressed.

The DESCRIPTION file

R code

jianhong commented 8 months ago

I forget the mention the Unit test. Important: Please improve the unit test coverage.

qfurumo commented 8 months ago

Thank you so much for looking through everything. I will get going on these revisions. One question I have is that I have been struggling to find good examples of unit test coverage online, and many guides say that tests are very specific to each data set. What kinds of test coverage are you looking for when it comes to this package? And if there are no specifics, what general approaches should I move towards?

jianhong commented 8 months ago

My cutoff for UNIT test is no lower than 30%. Sometimes, if you can cover the key functions will be also OK.

qfurumo commented 8 months ago

I apologize, this is the first time I have ever done formal unit testing or package submission so I am not familiar with what a 30% unit test cutoff would be. Also if I need to add additional testing to functions, is it permissible to go past the 50 line limit? Also is it common practice to create unit tests for entirely internal functions that the user does not have access to? Thank you for your help.

jianhong commented 8 months ago

about the test coverage, check here The unit test is used to verify the behavior of essential functions. You can start from the key functions in your package.

qfurumo commented 8 months ago

I apologize for the lapse in communication, I've had other grad student responsibilities take up a lot of time recently.

I had three questions: 1) For adding GRanges, is the general principal that GRanges will interact better with currently existing software? I read through the documentation and I don't know if there is much functionality that would be gained from rewriting the methods to integrate it. I understand if it is a necessary step but I want to be sure that I understand the important features I should be focusing on.

2) For the runnable examples in the Vignette, should there be full examples for all of methods (even those that are hidden from the user)? All of the methods except for one are internal, so do they need runnable examples if the user shouldn't need to access them in general?

3) Once I have the next draft of changes complete, do I just push it to my Git repository and notify you via this comment section?

Thank you for all your help and patience.

jianhong commented 8 months ago
  1. No need to change the major functions, but only inputs and outputs. The implementation of GRanges will enhance the users experiment with cooperative with other packages. Import bed file as GRanges is not difficult. You may want to take a look with rtracklayer package.
  2. The major functions. For example the step1, 2, 3 in your vignette.
  3. you need to push to git@git.bioconductor.org:packages/PIPETS
qfurumo commented 8 months ago

Hello,

I have just pushed a round of revisions to git@git.bioconductor.org:packages/PIPETS. I am still working on adding GRanges to the package so in the meantime I am pushing these changes for review as I continue to add it.

Responses to Notes and Comments from the review:

Output File Format: Will add a GRanges object output once GRanges is integrated.

Simpler Parameter Names: I have added a OutputFileID parameter to the base run file for the user to manually change. The parameters for the other inaccessible methods are currently unchanged because of code consistency on my end. If you expect users to interact with these inaccessible methods then I will happily change the parameters.

Consider shorter lines: The only lines that are longer than 80 characters are documentation lines that go into the ?method descriptions.

Remove paste in message(): All have been removed.

Keeping :: in source code: Utils is already on the import list in the description file. If this is unstable I can change it.

Vectorize Loops: There are no nested for loops so I don't think there is a lot of computational burden on keeping the for loops as is currently. I am much more familiar with for loops and I want to ensure that the code works as intended.

Remove Unused Code: Done

Consider adding drop = FALSE: I added it where I saw it applicable (in cases where dataframes were being directly changed as opposed to single vector/column changes).

Code Repetition: The method is inherently repetitive in that it is running almost the same functions on two different data sets derived from the input data. The input data is split into + and - strand and each has to be handled slightly differently. I am keeping the methods separate for both consistency as well as ability to fix errors.

Use BiocStyle for Vignette: Done

Add Abstract to Vignette: In process of writing journal article to go along with the method, will add abstract to the Vignette once that is complete.

Add runnable examples to Vignette: The three individual steps are all performed by the PIPETS_FullRun method and the individual methods that perform the steps are hidden from the user. I can add additional examples of the full run method but I am not sure that adding examples for methods the user cannot access would make things clearer.

Unit Testing: Improved unit testing and got testing coverage up to ~91%.

If there are any problems or questions please let me know. Thank you so much for your time and help.

jianhong commented 7 months ago

Hi @qfurumo Thank you for the updates. When making changes to your repository push to git@git.bioconductor.org:packages/PIPETS to trigger a new build. I will review code submitted to git.bioconductor.org only. A quick tutorial for setting up remotes and pushing to upstream can be found here.

qfurumo commented 7 months ago
GithubPushQuestion

I am not sure of a way to check on my end if everything worked but as far as the push attempt (screenshot attached) looks it seems to have worked?

lshep commented 7 months ago

Did you do a valid version bump to trigger the new build?

qfurumo commented 7 months ago
Screen Shot 2023-11-15 at 2 19 15 PM

I apologize, I am still pretty new to Git in general and I am having some difficulties with it. Up until today I have been able to push changes easily in Rstudio but now it has been requiring my github username and password every try. I have triple checked that I am entering the right values but it still denies authorization. I tried to make a temporary token/key and that also failed (in the screenshot). I am going to keep looking around to see what the problem is but if you have any advice/input that would be amazingly appreciated.

lshep commented 7 months ago

I don't know how you set up your remotes. origin normally is your github and upstream would be git.bioconductor.org if you followed our set up at http://contributions.bioconductor.org/git-version-control.html . From that point of view, Bioconductor does not have a 'master' branch you would need to push to 'devel'. Any ssh key created and trying to be used to access git.bioconductor.org must be uploaded to your BiocCredentials account.

bioc-issue-bot commented 7 months ago

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

qfurumo commented 7 months ago

The push seems to have gone through, I found the solution to the issue. Thank you for your help and patience.

bioc-issue-bot commented 7 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

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

Please see the build report for more details.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.2 LTS): PIPETS_0.99.0.tar.gz

Links above active for 21 days.

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

bioc-issue-bot commented 7 months ago

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

bioc-issue-bot commented 7 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

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

Please see the build report for more details.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.2 LTS): PIPETS_0.99.0.tar.gz macOS 12.7.1 Monterey: PIPETS_0.99.0.tar.gz

Links above active for 21 days.

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

jianhong commented 7 months ago

@qfurumo I think there is something wrong with the last push. Could you please bump your version number and push the updates again to the git.bioconductor.org?

bioc-issue-bot commented 7 months ago

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

bioc-issue-bot commented 7 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

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

Please see the build report for more details.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.2 LTS): PIPETS_0.99.0.tar.gz macOS 12.7.1 Monterey: PIPETS_0.99.0.tar.gz

Links above active for 21 days.

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

lshep commented 7 months ago

I honestly don't even know how you are able to trigger a build report as it is not suppose to trigger without a valid version bump; I'll have to debug this as it should not be the case. How and where are you pushing/what process are you doing? The last commit I see on git.bioconductor.org is from September 26?

qfurumo commented 7 months ago

I followed the procedure to link github with Rstudio using a video of a Bioconductor zoom workshop from a few years ago. I make the changes and commit and push all through Rstudio and I open the terminal in Rstudio and make sure that the changes are pushed upstream with "git push upstream". When I look at "git remote show origin" and "git remote show upstream" the addresses for the push locations match up with what they should be.

All of the changes have been pushed to my Github repository so I am pretty confused as to what is happening.

qfurumo commented 7 months ago

One thing I am seeing looking at Rstudio is that is says that I should be using the devel branch when I push to Bioconductor. I can't recall even setting my push directory to the master branch instead of devel. Could this be an issue and if so what is the best way to fix it?

jianhong commented 7 months ago

try {shell} git remote -v and paste the results here.

lshep commented 7 months ago

Yes you should be pushing to devel branch on git.bioconductor.org and it very well may be part of the problem. The master branches in package repositories will eventually be deleted on git.bioconductor.org and are only still there as packages make the transition. We no longer use master as a default branch it is devel in an effort to be more inclusive. Please see documentation here. If you started with your github as origin it will use whatever your default branch name is there; if you are still on that branch it will not change it when pushing to bioconductor. so git push upstream is equivalent to git push upstream master in this case. It would need to be set to do git push upstream master:devel (which says push this master branch to devel) or recommended is to rename all branches to devel to be consistent and then you can do straight forward git push, git push upstream.

qfurumo commented 7 months ago

Quinns-MacBook-Pro-5:PIPETS quinnfurumo$ git remote -v origin git@github.com:/qfurumo/PIPETS.git (fetch) origin git@github.com:/qfurumo/PIPETS.git (push) upstream git@git.bioconductor.org:packages/PIPETS.git (fetch) upstream git@git.bioconductor.org:packages/PIPETS.git (push)

bioc-issue-bot commented 7 months ago

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

bioc-issue-bot commented 7 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

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

Please see the build report for more details.

The following are build products from R CMD build on the Single Package Builder: macOS 12.7.1 Monterey: PIPETS_0.99.3.tar.gz Linux (Ubuntu 22.04.2 LTS): PIPETS_0.99.3.tar.gz

Links above active for 21 days.

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

lshep commented 7 months ago

Thank you! It looks like you were able to figure it out and the correct version is now on git.bioconductor.org and reflected in the build report.

qfurumo commented 7 months ago

Oh thank goodness ok. Honestly thank you both so much for all of the patience and help, I would have been in a much worse position trying to figure this out on my own.

jianhong commented 7 months ago

Package 'PIPETS' Review

Lots of progress!

Please try to answer the comments line by line when you are ready for a second review.

Code: Note: please consider; Important: must be addressed.

The DESCRIPTION file

General package development

R code

Documentation

qfurumo commented 6 months ago

Hello and Happy New Year.

Thank you for all of the suggestions and help. I am still working on making these updates and will be able to transition back to campus equipment and services this week so I hope to get these updates out soon.

Thanks.

bioc-issue-bot commented 6 months ago

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

bioc-issue-bot commented 6 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

On one or more platforms, the build results were: "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 build report for more details.

The following are build products from R CMD build on the Single Package Builder: macOS 12.7.1 Monterey: PIPETS_0.99.5.tar.gz

Links above active for 21 days.

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

bioc-issue-bot commented 6 months ago

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

bioc-issue-bot commented 6 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

On one or more platforms, the build results were: "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 build report for more details.

The following are build products from R CMD build on the Single Package Builder: macOS 12.7.1 Monterey: PIPETS_0.99.6.tar.gz

Links above active for 21 days.

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

bioc-issue-bot commented 6 months ago

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

bioc-issue-bot commented 6 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

On one or more platforms, the build results were: "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 build report for more details.

The following are build products from R CMD build on the Single Package Builder: macOS 12.7.1 Monterey: PIPETS_0.99.7.tar.gz

Links above active for 21 days.

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

bioc-issue-bot commented 6 months ago

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

bioc-issue-bot commented 6 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

On one or more platforms, the build results were: "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 build report for more details.

The following are build products from R CMD build on the Single Package Builder: macOS 12.7.1 Monterey: PIPETS_0.99.8.tar.gz

Links above active for 21 days.

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

qfurumo commented 6 months ago

Hello, As all of these push attempts would indicate, I am stuck on an error I haven't encountered and cannot generate on my end using BiocCheck. The error is:

Checking package installation calls in R code... Error in .getDirFiles(rowdf["filename"]) :

'fpaths' input must be a character vector Calls: BiocCheck ... findSymbolsInParsedCode -> apply -> FUN -> sprintf -> .getDirFiles I have been googling it and related errors to no avail, I was wondering if you have seen this before or if you could point me in the right direction for solving it.
lshep commented 6 months ago

@LiNk-NY ?

LiNk-NY commented 5 months ago

Hello, As all of these push attempts would indicate, I am stuck on an error I haven't encountered and cannot generate on my end using BiocCheck. The error is:

Checking package installation calls in R code... Error in .getDirFiles(rowdf["filename"]) :

'fpaths' input must be a character vector Calls: BiocCheck ... findSymbolsInParsedCode -> apply -> FUN -> sprintf -> .getDirFiles I have been googling it and related errors to no avail, I was wondering if you have seen this before or if you could point me in the right direction for solving it.

This has been resolved. Please bump the version for another build.

Best regards, Marcel

bioc-issue-bot commented 5 months ago

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