Bioconductor / Contributions

Contribute Packages to Bioconductor
133 stars 33 forks source link

magpie #2898

Closed dxd429 closed 1 year ago

dxd429 commented 1 year 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 1 year ago

Hi @dxd429

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: magpie
Type: Package
Title: MeRIP-Seq data Analysis for Genomic Power Investigation and Evaluation
Version: 0.99.0
Authors@R: c(person(given = "Daoyu",
          family = "Duan",
          role = c("aut", "cre"),
          email = "dxd429@case.edu"),
   person(given = "Zhenxing",
          family = "Guo",
          role = c("aut"),
          email = "guozhenxing@cuhk.edu.cn"))
Description: This package aims to perform power analysis for the MeRIP-seq study. It calculates FDR, FDC,
 power, and precision under various study design parameters, including but not limited to 
 sample size, sequencing depth, and testing method. It can also output results into .xlsx files or 
 produce corresponding figures of choice. 
License: MIT + file LICENSE
Encoding: UTF-8
biocViews: Epitranscriptomics, DifferentialMethylation, Sequencing, RNASeq, Software
RoxygenNote: 7.2.1
Imports: 
    utils,
    rtracklayer,
    Matrix,
    matrixStats,
    stats,
    S4Vectors,
    methods,
    graphics,
    GenomicRanges,
    GenomicFeatures,
    IRanges,
    Rsamtools,
    AnnotationDbi,
    aod,
    BiocParallel,
    DESeq2,
    openxlsx,
    RColorBrewer,
    reshape2,
    TRESS
Depends: R (>= 4.1.0)
Suggests: 
    knitr,
    rmarkdown,
    kableExtra
VignetteBuilder: knitr
URL: https://github.com/dxd429/magpie
BugReports: https://github.com/dxd429/magpie/issues
bioc-issue-bot commented 1 year ago

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 commented 1 year 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, 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 git@git.bioconductor.org:packages/magpie to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

lshep commented 1 year ago

This ERROR is quite concerning if you are trying to download anything from CRAN it should be in an eval=FALSE section or dontrun . You should never install a package for a user without their knowledge.

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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". 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 git@git.bioconductor.org:packages/magpie to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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". 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 git@git.bioconductor.org:packages/magpie to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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". 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 git@git.bioconductor.org:packages/magpie to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

dxd429 commented 1 year ago

Hi, I already subscribed to the mailing list, but still got the error: "ERROR: Maintainer must subscribe to the bioc-devel mailing list."

ococrook commented 1 year ago

Hi @dxd429

Don't worry about that error, I'll have a look into it.

ococrook commented 1 year ago

Hi @dxd429

This package looks quite good, thanks for the submission

Minor:

General comments:

Please add a Readme it's currently not clear what the package is trying to do without going into the package

There is quite a lot of code that is undocumented or uncommented, it's hard to workout what each function does without at least a description of its intended use. I dont expect the same quality of description as exported code but some explaination would help. The code could also benifit from more white space and starting function arguments on new lines

Is there any need for the LICENSE file - what is this doing in addition to the MIT license?

dxd429 commented 1 year ago

Hi @dxd429

This package looks quite good, thanks for the submission

Minor:

  • [ ] Please update the R dependency
  • [ ] Please add units tests

General comments:

Please add a Readme it's currently not clear what the package is trying to do without going into the package

There is quite a lot of code that is undocumented or uncommented, it's hard to workout what each function does without at least a description of its intended use. I dont expect the same quality of description as exported code but some explaination would help. The code could also benifit from more white space and starting function arguments on new lines

Is there any need for the LICENSE file - what is this doing in addition to the MIT license?

Hi Oliver,

Thank you for your comments! I will address minors and your comments accordingly. Regarding the LICENSE file, I think it was generated accidentally and will remove it. Once updated, I will push it again.

dxd429 commented 1 year ago

Hi @dxd429

This package looks quite good, thanks for the submission

Minor:

  • [ ] Please update the R dependency
  • [ ] Please add units tests

General comments:

Please add a Readme it's currently not clear what the package is trying to do without going into the package

There is quite a lot of code that is undocumented or uncommented, it's hard to workout what each function does without at least a description of its intended use. I dont expect the same quality of description as exported code but some explaination would help. The code could also benifit from more white space and starting function arguments on new lines

Is there any need for the LICENSE file - what is this doing in addition to the MIT license?

Hi Oliver,

About the unit test, I was trying to submit an Experimental Data package to store the data for main function testing. I tried to include 8 small data files, but the largest data file is still 11.3 MB which prevents me from submitting it on Bioconductor. Can I run the unit test with the Data package published on GitHub instead?

ococrook commented 1 year ago

Sorry for the slow reply - for some reasons I didn't get a notification.

Unit tests shouldn't really rely on large data, and should be as small as possible. It's best to try to make the data smaller or have simulations where you know what the output should be and can see any errors quickly.

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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". 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 git@git.bioconductor.org:packages/magpie to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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. 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 git@git.bioconductor.org:packages/magpie to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

dxd429 commented 1 year ago

Sorry for the slow reply - for some reasons I didn't get a notification.

Unit tests shouldn't really rely on large data, and should be as small as possible. It's best to try to make the data smaller or have simulations where you know what the output should be and can see any errors quickly.

Hi Oliver,

I addressed the comments you raised, and now it passed the check with no errors:

However, I was not able to find a .BAM file that is less than 5MB for unit test, so there is no unit test for now. Is a unit test required here?

ococrook commented 1 year ago

Thanks! Looks good

I think we should check with @lshep on the unit test front. Personally, I'm anxious that bugs can accumulate easily without units tests.

@lshep the author is finding it hard to obtain suitable data for unit testing. I suggested trying to make the data smaller or using simulated data but they don't seem to find it possible. Can we exceed the 5mb limit in this case? Or do you have another way to proceed? Thanks!

lshep commented 1 year ago

The 5mb limit is a hard limit and cannot be removed. Would a BAM file from the AnnotationHub be usable?

dxd429 commented 1 year ago

The 5mb limit is a hard limit and cannot be removed. Would a BAM file from the AnnotationHub be usable?

Hi, thank you for replying, but it depends on the type of data. I made an ExperimentData package which contains the .BAM files needed for purposes like unit test, but still was not able to pass Biocheck because of their sizes. But I did see some ExperimentData packages containing files larger than 5MB, so is this a doable way?

I will try to find a published ExperimentData packge with .BAM files if I cannot publish mine.

Thank you!

lshep commented 1 year ago

ExperimentHub packages do not have the data contained in them directly. The Data files are stored on a server and ExpeirmentHub points to the data to download.

dxd429 commented 1 year ago

ExperimentHub packages do not have the data contained in them directly. The Data files are stored on a server and ExpeirmentHub points to the data to download.

Thank you for the information! But I checked some "ExperimentData" packages containing .BAM files, for example, "SCLCBam" and "MMDiffBamSubset". They put large (50 - 100 MB) .BAM files in "inst/extdata/" folder instead of a server. A ExperimentHub package should also have "metadata.csv" in that folder as I checked, but I do not see it in theirs.

If a unit test is required here, may I have any suggestions on using files that are at most 12 MB in the test? My package utilizes .BAM files from MeRIP-Seq experiments, which are not available in any published package for now.

Thank you!

lshep commented 1 year ago

These were legacy experiment data packages before we transitioned to using git and tracking experiment data packages.

dxd429 commented 1 year ago

These were legacy experiment data packages before we transitioned to using git and tracking experiment data packages.

I see. Then for the unit test, can I use data from the data package on my GitHub instead? If not, should I make an ExperimentHub package for the unit test?

Thank you!

lshep commented 1 year ago

We do not allow data to be downloaded and used from github it would have to be on a trusted server like zenodo, aws, etc. Again, if you are finding BAM files in other packages why not just use the ones that are already available then?

dxd429 commented 1 year ago

We do not allow data to be downloaded and used from github it would have to be on a trusted server like zenodo, aws, etc. Again, if you are finding BAM files in other packages why not just use the ones that are already available then?

The .BAM files from other packages are from RNA-Seq experiments, not MeRIP-Seq which is what my package is designed for. But I guess they could work since it is just for the unit test. Thank you for suggesting and I will try and see if it works.

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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. 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 git@git.bioconductor.org:packages/magpie to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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. 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 git@git.bioconductor.org:packages/magpie to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

dxd429 commented 1 year ago

Thanks! Looks good

I think we should check with @lshep on the unit test front. Personally, I'm anxious that bugs can accumulate easily without units tests.

@lshep the author is finding it hard to obtain suitable data for unit testing. I suggested trying to make the data smaller or using simulated data but they don't seem to find it possible. Can we exceed the 5mb limit in this case? Or do you have another way to proceed? Thanks!

Hi Oliver,

I was finally able to use one of the Bioconductor package "TBX20BamSubset" to perform the unit test. I added the unit test but got a warning saying "R CMD check exceeded 10 min requirement" on one of the platform.

As I checked, one option for me is "long test", but it says "The Long Tests setup forces developers to split the testing code in their package between “short tests” and “long tests”. The former go in the tests subdirectory and must be able to run in less than 40 min" on the Bioconductor page. my unit test runs in around 3 mins, so short test should be good here.

Do you have any suggestions? Thank you in advance!

lshep commented 1 year ago

I think it will be okay to proceed despite this warning.

dxd429 commented 1 year ago

@ococrook Hi, is the package ready for next step of the review process?

ococrook commented 1 year ago

Could you align the package versions between the NEWS.md and the Description and then I can accept the package

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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. 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 git@git.bioconductor.org:packages/magpie to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

dxd429 commented 1 year ago

Could you align the package versions between the NEWS.md and the Description and then I can accept the package

@ococrook Hi, I aligned the version numbers and it was built successfully.

ococrook commented 1 year ago

Brilliant, thank you!

bioc-issue-bot commented 1 year ago

Your package has been accepted. It will be added to the Bioconductor nightly builds.

Thank you for contributing to Bioconductor!

Reviewers for Bioconductor packages are volunteers from the Bioconductor community. If you are interested in becoming a Bioconductor package reviewer, please see Reviewers Expectations.

lshep commented 1 year ago

The default branch of your GitHub repository has been added to Bioconductor's git repository as branch devel.

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/dxd429.keys is not empty), then no further steps are required. Otherwise, do the following:

  1. Add an SSH key to your github account
  2. Submit your SSH key to Bioconductor

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("magpie"). The package 'landing page' will be created at

https://bioconductor.org/packages/magpie

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.