Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

Please add package mbQTL #2899

Closed Mercedeh66 closed 1 year ago

Mercedeh66 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 @Mercedeh66

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: mbQTL
Type: Package
Title: mbQTL
Version: 0.99.0
Authors@R: 
    c(person(given = "Mercedeh",
 family = "Movassagh",
 role = c("aut", "cre"),
 email = "mercedeh.movassagh@yale.edu",
 comment = c(ORCID = "0000-0001-7690-0230")),
 person("Steven","Schiff", role="aut"),
 person("Joseph N", "Paulson", role="aut"))
Description: mbQTL is a statistical R package for simultaneous 16srRNA,16srDNA (microbial)
 and variant, SNP, SNV (host) relationship, correlation, regression studies.
 We apply linear, logistic and correlation based statistics to identify the relationships
 of taxa, genus, species and variant, SNP, SNV in the infected host. We produce various 
 statistical significance measures such as P values, FDR, BC and probability estimation to
 show significance of these relationships. Further we provide various visualization function
 for ease and clarification of the results of these analysis. The package is compatible with
 dataframe, MRexperiment and text formats.
License: MIT + file LICENSE
Encoding: UTF-8
Depends: R (>= 4.1.0), BiocStyle
DeploySubPath: mbQTL
biocViews: SNP, Microbiome,WholeGenome, Metagenomics, StatisticalMethod, Regression
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.2.1
Imports:
  MatrixEQTL,
  dplyr,
  ggplot2,
  readxl,
  stringr,
  tidyr,
  metagenomeSeq,
  pheatmap,
  broom,
  graphics,
  stats
Suggests: 
    knitr,
    rmarkdown
VignetteBuilder: knitr
vjcitn commented 1 year ago

Please run R CMD check on your package

Running examples in 'mbQTL-Ex.R' failed
The error most likely occurred in:

> ### Name: allToAllProduct
> ### Title: allToAllProduct creates a dataframe of snp and taxa correlations
> ### Aliases: allToAllProduct
> ### Keywords: Correlation snptaxa
> 
> ### ** Examples
> 
> x <- allToAllProduct(SnpFile, microbeAbund, "chr1.171282963_T")
Error in h(simpleError(msg, call)) : 
  error in evaluating the argument 'x' in selecting a method for function 'rownames': object 'microbeAbund' not found
Calls: allToAllProduct -> RegSnp -> rownames -> .handleSimpleError -> h
Execution halted
* checking for unstated dependencies in vignettes ... OK
Mercedeh66 commented 1 year ago

Hi thank you for spotting this. I have corrected the errors. Please try again. Thank you!

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". 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/mbQTL to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

Mercedeh66 commented 1 year ago

@HelenaLC should be good to go! Thanks.

HelenaLC commented 1 year ago

Some (mostly minor) comments below! Note that many are suggestions and not strictly required (e.g., "consider..."). Please comment back here with what has/not been addressed how/why not, or any comments regarding any needed clarification or questions you might have; happy to help/discuss. Cheers!

DESCRIPTION

code

data

documentation

vignette

Mercedeh66 commented 1 year ago

Hi @HelenaLC. Tried to address almost all of the comments. Many thanks for the guidance.

Best,

-M

DESCRIPTION Would be nice to add a brief but descriptive Title: for the package. This will show up on the Bioconductor website (here's an example of what I mean). - Done

As mentioned in the build report, please update Depends: from R v4.2 to 4.3 (since anyways Bioc 3.17 won't be installable under older versions of R). -Done

code I'd strongly suggest adding some argument validity checks with informative messages to the user. For example, say your function expects a data.frame with rownames set an 2 non-zero dimensions and numeric values (e.g., coringTaxa()), users might get obstruct errors if this it not the case. -Done I understand using dplyr and tidyr is nice on a daily basis, but it adds dozens of dependencies for functionality that can often be achieved using base R. If at all possible, I'd try working around these. Of course, that's left to your personal judgement; but it it's just about mutate, filter, select and friends, I'd argue the cost (in terms of dependencies) is not worth the benefit. -I agree, in this particular case we will be dealing with very large snp files and taxa files and processing is easier with dplyr so I am going to stick with it. Thanks for the valuable advice though.

data All that's in data-raw at the moment should go in an inst/extdata directory, and be read into R with system.file() (see here for details). Accordingly, the mydataset.R script would go in inst/script as described here. -Done Please properly document all example data in the corresponding man pages, and provide source information and/or code (in inst/script) on how the example data were generated. There's virtually no description of the data at the moment. -Improved-Done

documentation (more of a question than a comment ;) The NEWS states v0.99.1, but the DESCRIPTION is at v0.99.0. Make sure to update the latter and push version bumps to the local (origin) as well as to Bioc (upstream) branch when making updates. -Done Nothing to complain about really, but did you know you can write data(one, two, three) rather than having multiple data() calls? Perhaps something worth considering to keep examples more compact. -Thanks Please including Bioconductor installation instructions using BiocManager::install() in the READEME. -Done (optional) We would highly encourage having a package man page so users would be directed to the main functions of the package with ?mbQTL. I'd recommend against importing dependencies in their entirety with @import , and rather use selection imports via @importForm . Ideally, these go in the roxygen chunk preceding the corresponding function that uses them. This makes it clear which namespace functions are being called from and avoid namespace conflicts (say, two packages exporting functions of the same name).

We don't allow \dontrun in examples unless there is a strong justification for this (?) Currently in Logistic_Regression.R (lines 27,62) and Correlation_Regression.R (line 67) -Done For all functions, I feel that the documentation could be a little more extensive (according to your own judgement). E.g., when there's a statistical test/modeling step, pointers to the underlying methods would be nice (e.g., function hyperlinks). Or a brief @details section that describes what is being done. Similarly, under @return, it'd be nice to briefly describe the different columns (with their explicit name). To give a simple example, rather than saying "MatrixeQR core functions are utilized to achieve this", it would be nice to be explicit about which function (e.g., Matrix_eQTL_engine()) is used and for what. Note than, I can see this in the code but 'normal' users cannot without direct pointers. -Done vignette We encourage the use of r BiocStyle::Biocpkg("BiocStyle") for formatting with html_document as rendering target; see here for instructions. You have it under Suggests: in the DESCRIPTION, but it's currently not being used? -Done To highlight/distinguish code (e.g., package/function/variable/class names) from normal text, I'd highly recommend consistently using back-ticks. Makes it a lot easier to read/follow things. -Changed format. Especially in the vignette, please stick to a 80-character limit as code might be otherwise hard to read when displayed online (which it typically is when accessing the HTML of the vignette on the Bioc website). -Done Besides the intro giving some background, the vignette currently gives few details on the methods being run and outputs produced. For example, rather than just running through the examples, briefly explain the underlying methods; perhaps display chunks of the output and explain what they mean/how they should be interpreted; point to relevant outside packages/functions (e.g., using BiocStyle functions for hyperlinks) etc. Overall, the vignette is a little sparse right now. -Improved it.

HelenaLC commented 1 year ago

So, looks like your origin branch DESCRIPTION here is at v0.99.2, but the Bioc one is still at 0.99.0. You need to (now and for all time to come as a Bioc maintainer) push to both the local (origin) and bioc branch (upstream) to trigger a new build. Also note that this will trigger the Bioc bot to post a message here that a "new build has been triggered".

Also, would be nice if you could edit the above (ideally, edit, not repost, to keep the thread clean) to keep the .md formatting and code highlighting from my original comments. E.g., you can do a "Quote reply" on GitHub to reply to the original post.

bioc-issue-bot commented 1 year ago

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

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/mbQTL 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: 89ccdf97e7c1c157ad196d9190db682f8136b9d8

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/mbQTL 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: 15888db7d9957a0b2270d2599580eaf93f0ef48c

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/mbQTL to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

Mercedeh66 commented 1 year ago

So, looks like your origin branch DESCRIPTION here is at v0.99.2, but the Bioc one is still at 0.99.0. You need to (now and for all time to come as a Bioc maintainer) push to both the local (origin) and bioc branch (upstream) to trigger a new build. Also note that this will trigger the Bioc bot to post a message here that a "new build has been triggered".

Also, would be nice if you could edit the above (ideally, edit, not repost, to keep the thread clean) to keep the .md formatting and code highlighting from my original comments. E.g., you can do a "Quote reply" on GitHub to reply to the original post.

Just wanted to let you know, all is done and it is passing the check now.

HelenaLC commented 1 year ago

Hey Mercedeh, see my comments below.

Kindly, could you respond using .md syntax (or a quoted response) this time? It's quite painful to read unformatted plain text bullet lists and code on GH...

bioc-issue-bot commented 1 year ago

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

Mercedeh66 commented 1 year ago

Hey Mercedeh, see my comments below.

Kindly, could you respond using .md syntax (or a quoted response) this time? It's quite painful to read unformatted plain text bullet lists and code on GH...

  • in the LICENSE: Copyright (c) 2022 mirTarRnaSeq authors -> ...2023 mbQTL... DONE
  • in the DESCRIPTION: consider adding a URL: and BugReports: field pointing to "https://github.com/Mercedeh66/mbQTL" and ".../issues", respectively DONE
  • perhaps add a space in the DESCRIPTIONs Title: mbQTL:A package before the "A"? DONE
  • some typos I found in the vignette (perhaps worth giving everything another read...):

    • line 95: vigette -> vignette DONE
    • 100: add space b/w "formats.The" DONE
    • 113: aggreagate -> aggregated, taxanomic -> taxonomic (I think?), DONE
    • 114: formated -> formatted DONE
    • 131: hetrozygous -> heterozygous DONE
    • 166: abundancefile -> abundance file DONE
  • also in the vignette: across all code chunks, I'd recommend not exceeding the 80-char limit, even with comments (this might look messy when displayed online...) I am assuming you mean per line limit. Altered the best I could.
  • have you considered this...? "having a package man page so users would be directed to the main functions of the package with ?mbQTL." (double checking because you didn't comment on / implement this) Added Done
  • in the vignette, re "to highlight/distinguish code [...]": you said "Done", but everything is still in plain text. I find this very difficult to read / follow; especially when referring to variable/function/package names, code-style would improve readability a lot (potentially adding hyperlinks via BiocStyle commands when referring to external software) Done
  • perhaps I'm missing something, but I can still not find information on data source/reproducibility. I can see that there are comments in inst/script/mydataset.R, but these will not be visible to regular users. Instead, there should be a man page containing these information (accessible via ?mbQTL::data or, e.g., ?CovFile etc.). All that is to say: data documentation should go in the roxygen chunks of your code/data.R script. This is strange I thought I updated this in the data.R file and it should now be there please try again. If I am missing something please let me know.

Thanks so much Helena for such an in depth and detailed review and the great comments/suggestions. I really appreciate it. I hope all is good now.

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/mbQTL 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

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

https://bioconductor.org/packages/mbQTL

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.