MSstatsBioNet - Githubissues

tonywu1999 commented 4 months ago

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

Repository: https://github.com/Vitek-Lab/MSstatsBioNet

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.

bioc-issue-bot commented 4 months ago

Hi @tonywu1999

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: MSstatsBioNet
Type: Package
Title: Network Analysis for MS-based Proteomics Experiments
Version: 0.99.0
Authors@R: c(
    person("Anthony", "Wu", email = "wu.anthon@northeastern.edu", 
        role = c("aut", "cre")),
    person("Olga", "Vitek", email = "o.vitek@northeastern.edu", 
        role = "aut", comment=c(ORCID = "0000-0003-1728-1104"))) 
Description: A set of tools for network analysis using mass 
    spectrometry-based proteomics data and network databases. The package takes 
    as input the output of MSstats differential abundance analysis and provides 
    functions to perform enrichment analysis and visualization in the 
    context of prior knowledge from past literature. Notably, this package 
    integrates with INDRA, which is a database of biological networks
    extracted from the literature using text mining techniques.
License: Artistic-2.0
Depends: R (>= 4.4.0)
Imports: 
    RCy3,
    httr,
    jsonlite,
    r2r
Suggests: 
    data.table,
    BiocStyle,
    knitr,
    rmarkdown,
    testthat (>= 3.0.0),
    mockery
VignetteBuilder: knitr
biocViews: ImmunoOncology, MassSpectrometry, Proteomics, Software, 
    QualityControl, NetworkEnrichment, Network
Encoding: UTF-8
URL: http://msstats.org, https://vitek-lab.github.io/MSstatsBioNet/
BugReports: https://groups.google.com/forum/#!forum/msstats
Config/testthat/edition: 3
RoxygenNote: 7.3.2

tonywu1999 commented 3 months ago

Is there any ETA for the next step?

lshep commented 3 months ago

We try to have progress within 2 weeks. We are a little behind from the annual Bioconductor conference that occurred last week. We will aim to do another processing of packages and you can expect either feedback or it to be moved into the next steps by next week.

lshep commented 3 months ago

INDRA is licensed under a different license BDS 2-clause license , and per that licensing agreement says that if used it has to be distributed with and reference the license. If you are licensing your package differently you should make note of the licensing difference in the README and vignette; and if that function is used a message indicating that the licensing agreement must be included with any results. It may be necessary to also include its license in your package.

Please use the standard directory inst/extdata for including additional data referenced in man/vignette with system.file. You should also include an inst/script directory that describes how this data was created. It can be text, pseudo code, or code but minimally contain reference/source/licensing information. A user should have an idea of how to recreate these files if desired. Reading the vignette, it seems like this should be run in conjunction with the MSstats package. I'm curious as to why extra data is needed at all? If that is the case shouldn't you be able to use the examples and function in the MSstats package directly in your package? I'm currently failing to see the connection to the package; if you could please expand the vignette.

You current vignette is inadequate as there are no functions run. All Bioconductor vignettes must have runnable code to ensure a properly running package. Similarly none of the code is run in the examples. Your functions that you export must be run at some point to ensure they are properly running and connecting as intended.

tonywu1999 commented 3 months ago

You current vignette is inadequate as there are no functions run. All Bioconductor vignettes must have runnable code to ensure a properly running package. Similarly none of the code is run in the examples. Your functions that you export must be run at some point to ensure they are properly running and connecting as intended.

Both of my functions rely on external dependencies that need to be available. For example, getSubnetworksFromIndra requires internet access to call INDRA, while the other requires the Cytoscape Desktop app to be open. So far, I've added unit tests with mock calls to INDRA/Cytoscape to handle testing these functions.

As of now, if I run these functions without internet access or without the Cytoscape app open within a vignette, they fail, which is a likely scenario when running the build on a remote server (e.g. building on a bioc remote server).

Do you have any guidance on how other packages approach these situations?

hpages commented 2 months ago

Our build machines have internet access. Generally speaking it's ok to assume that internet is available in your vignettes and man page examples.

As for access to the Cytoscape Desktop app, unfortunately we cannot assume that it's available. Hopefully this is not essential to your vignette or man pages.

Please use <- instead of = for assignment. Thanks!

vjcitn commented 2 months ago

It is good that you have mock data solution. Protect the internet/app-dependent code with if (interactive()) the build system is non-interactive and will skip the dependent code.

tonywu1999 commented 2 months ago

Our build machines have internet access. Generally speaking it's ok to assume that internet is available in your vignettes and man page examples. As for access to the Cytoscape Desktop app, unfortunately we cannot assume that it's available. Hopefully this is not essential to your vignette or man pages. Please use <- instead of = for assignment. Thanks!

Updated the vignettes and docs to have zero comments now. I also changed to the <- assignment operator

It is good that you have mock data solution. Protect the internet/app-dependent code with if (interactive()) the build system is non-interactive and will skip the dependent code.

Thank you for that suggestion. I updated the function involving Cytoscape Desktop to only execute if interactive() is true. I also unit tested cases where interactive() is true with mocks.

tonywu1999 commented 2 months ago

I believe I am ready for the next steps given all comments have been addressed.

bioc-issue-bot commented 2 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 2 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: "skipped, 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: Linux (Ubuntu 24.04.1 LTS): MSstatsBioNet_0.99.2.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/MSstatsBioNet to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

lshep commented 2 months ago

Please fix the support site error seen on teran2 report and kick off a new build report with a valid version bump before a reviewer is assigned

bioc-issue-bot commented 2 months ago

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

bioc-issue-bot commented 2 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: Linux (Ubuntu 24.04.1 LTS): MSstatsBioNet_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/MSstatsBioNet to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 2 months ago

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

bioc-issue-bot commented 2 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 24.04.1 LTS): MSstatsBioNet_0.99.4.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/MSstatsBioNet to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 2 months ago

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

DarioS commented 2 months ago

Vignette does not use BiocStyle. See Vignettes section of the developer's guide.
Vignette lacks installation section near the beginning. See Installation section of user guide.
Vignette has code chunks but lacks interpretation after them. Add one or two sentences explaining the results.
Inconsistent use of snake_case and camelCase variable styles. Please use camelCase according to developer's guide.
Function documentation is inadequate. \description is just a copy-and-paste of the title. Please add proper descriptions.

\title{Get subnetwork from INDRA database with differential analysis results}
    ...        ...
\description{Get subnetwork from INDRA database with differential analysis results}

Also, what is a "comparisionResult table"? You need to use \seealso to cross-link and provide context. "node: dataframe of edges". What are the required column names and data types of each column?

bioc-issue-bot commented 1 month ago

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

bioc-issue-bot commented 1 month 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 24.04.1 LTS): MSstatsBioNet_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/MSstatsBioNet to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

tonywu1999 commented 1 month ago

@DarioS

Addressed your comments. Thanks!

DarioS commented 1 month ago

The package has been improve. A couple of issues remain.

input: Output of groupComparison function's comparisionResult table ...

I still don't know where groupComparison comes from. Please use \link according to Cross-references.

Manually writing out protein ID conversions is inadequate.

uniprot_to_hgnc_mapping <- c(
    "B9A064" = "38476",
    "O00391" = "9756",
    "O14818" = "9536",
    "O43598" = "21218",
    "O43707" = "166",
    "P16050" = "11390",
    "P84243" = "4765"
)

Please demonstrate improved Bioconductor interoperability of your package by using UniProt.ws to do it.

The package makes use of UniProt's modernized REST API and allows mapping of identifiers accross different databases.

tonywu1999 commented 4 weeks ago

@DarioS Could you clarify why this cross-reference is not sufficient? Is there anywhere else where I need to add a cross-reference?

For the ID conversion, we have this PR that converts the IDs based on mappings from INDRA. Once this PR is merged, I will update the vignettes and update this thread.

DarioS commented 4 weeks ago

Ah, that is O.K. But it is typically done in the parameter description. See ?findOverlaps when GenomicRanges is loaded. Notice how the links are all in the argument descriptions and this is a Bioconductor core-developed package. However, I think MSstats should be in Depends: not in Suggests: MSStatsBioNet takes as input the output of MSStats, so it is a core dependency.

Please update your DESCRIPTION file accordingly.

bioc-issue-bot commented 3 weeks ago

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

bioc-issue-bot commented 3 weeks 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: "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.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 24.04.1 LTS): MSstatsBioNet_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/MSstatsBioNet to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

Bioconductor / Contributions

MSstatsBioNet #3500