ReactomeGraph4R - Githubissues

chilampoon commented 3 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/reactome/ReactomeGraph4R

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

bioc-issue-bot commented 3 years ago

Hi @chilampoon

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: ReactomeGraph4R
Title: R Interface for the Reactome Graph Database
Version: 0.99.0
Authors@R: c(
    person("Chi-Lam", "Poon", role = c("aut", "cre"), email = "clpoon807@gmail.com", 
  comment = c(ORCID = "0000-0001-6298-7099")),
    person("Reactome", role = "cph", email = "help@reactome.org"))
Description: Pathways, reactions, and biological entities in Reactome knowledge 
    are systematically represented as an ordered network and stored in the 
    Reactome Graph Database. This package provides an interface to query 
    interconnected data in the local Neo4j database.
License: file LICENSE
Encoding: UTF-8
URL: https://github.com/reactome/ReactomeGraph4R
BugReports: https://github.com/reactome/ReactomeGraph4R/issues
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.1.1
Imports:
    neo4r (>= 0.1.3),
    utils,
    getPass,
    jsonlite,
    purrr,
    magrittr,
    data.table,
    rlang,
    ReactomeContentService4R,
    doParallel,
    parallel,
    foreach
Suggests: 
    knitr,
    rmarkdown,
    testthat,
    stringr,
    networkD3,
    visNetwork,
    wesanderson
VignetteBuilder: knitr
biocViews: DataImport, Pathways, Reactome, Network, GraphAndNetwork

bioc-issue-bot commented 3 years 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 3 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: "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. 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/ReactomeGraph4R to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

lshep commented 3 years ago

The neo4r package is only available on CRAN for 0.1.1. 0.1.3 is not available. https://cran.r-project.org/web/packages/neo4r/index.html . Bioconductor only uses packages available on CRAN / Bioconductor . Either remove the version requirement or you will not be able to proceed until the newer version of the package is available on CRAN.

bioc-issue-bot commented 3 years ago

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

bioc-issue-bot commented 3 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: "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. 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/ReactomeGraph4R to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

chilampoon commented 3 years ago

@vjcitn this package needs to connect to a local database first (described here: https://github.com/reactome/ReactomeGraph4R). How can I pass the checks on bioconducotr builders?

vjcitn commented 3 years ago

Thank you for this interesting package. The infrastructure required on your README is significant and I would suggest using "mocks" to allow your package into the ecosystem. I don't think we will install neo4j on our build systems. But you can have some intermediate objects of modest size that behave like the graph database records and can be used to test your R code. I will check with others about the requirements but I think your best bet is not to require the neo4j deployment. We are managing 1900 packages and yours is the first with this requirement. @hpages ?

chilampoon commented 3 years ago

@vjcitn Thanks for your response! Is something like this what you mean by "mocks"?

vjcitn commented 3 years ago

It seems relevant. Do you have functions to ingest that text into R and perform graph operations on it, perhaps with igraph?

chilampoon commented 3 years ago

I only have functions to ingest json data returned from neo4j. One way to pass the check is to \dontrun all exported query functions... I think that's also what neo4r did. And maybe to store some R objects then test other processing functions?

vjcitn commented 3 years ago

Let's not rush into this. I will spend some time with your package and get back to you.

vjcitn commented 3 years ago

I ran through your vignette. It was easy to do this. It required 4.7GB for the neo4j data, i guess. Is that something you will be updating on a regular basis? Users must acquire the neo4j infrastructure and connect to it. It seems much heavier than anything we have done with reactome or perhaps any other package so far. We have had a static reactome.db package for a long time, that gets updated each release. I see on the reactome github that there is a REST API. It seems to me that that could be a simple way of providing reactome data to Bioc users. Can you expose a neo4j instance to of reactome to the web?

vjcitn commented 3 years ago

The ReactomeGSA package uses gsa.reactome.org ... so the REST API is being used. We will discuss what would be involved with introducing this to the build system.

In trying out an example, I found it interesting that you manage the connection as a global option

> options()$con
<neo4j connection object>
Connected at http://localhost:7474 
User: neo4j 
Neo4j version: 3.5.19 
>

That's not a good practice. IMHO the connection object should have local state and be passed explicitly when needed.

What I meant by "mock" is somewhat vague, but let's take

"MATCH p1 = (disease:Disease)<-[:disease]-(dbo:DatabaseObject) WHERE disease.displayName = \"asthma\" RETURN disease,dbo,relationships(p1)"

You are creating that in R, are you validating it in R before passing it to neo4j? If so testing that validation step would be relevant, and presumably you don't need neo4j running to validate it. You won't know whether the disease token is valid so you can't test that without neo4j. But that string is an example of a mock datum that is usable for testing package functionality without contacting the server. Another example would be to save some of the tables you generate, and carry out the visualizations proposed in the vignette.

I'll get back to you on the idea of hosting the package in the Bioc build system.

chilampoon commented 3 years ago

gsa.reactome.org is the Analysis Service API which is different from the data query API (Content Service, https://reactome.org/ContentService/). There's also a wrapper for the ContentService API and it has been submitted (https://github.com/reactome/ReactomeContentService4R)... Data retrieved from the Graph Database are mostly with hierarchical/network structure and contain the node and relationship information, these are what you cannot get from the data from the REST API or that reactome.db (I am not so sure but this package looks like a hub for annotation files)

vjcitn commented 3 years ago

Have you considered placing a small slice of reactome in neo4j aura -- you might pay something per month -- for the sake of testing and demonstration? The bioc package code could use that small instance to do its testing. Then interested users could follow the instructions to get the full deployment, which is quite convenient for docker users.

chilampoon commented 3 years ago

Neo4j aura seems convenient but ummm I have to ask the Reactome team

Have you considered placing a small slice of reactome in neo4j aura -- you might pay something per month -- for the sake of testing and demonstration? The bioc package code could use that small instance to do its testing. Then interested users could follow the instructions to get the full deployment, which is quite convenient for docker users.

chilampoon commented 3 years ago

@vjcitn I think we can just use mocks, and I'll think about what stuff to be tested. I actually called the con in my query and error checking functions, I am not sure how to store it only in the package environment, do you have any suggestions?

> options()$con
<neo4j connection object>
Connected at http://localhost:7474 
User: neo4j 
Neo4j version: 3.5.19 
> 
That's not a good practice. IMHO the connection object should have local state and be passed explicitly when needed.

vjcitn commented 3 years ago

You don't have to store it in the package environment. You have a constructor that produces an object that holds the connection. You could look at bigrquery for an example

>    if (bq_testable()) {
+      con <- DBI::dbConnect(
+        bigquery(),
+        project = "publicdata",
+        dataset = "samples",
+        billing = bq_test_project()
+      )
+      con
+      DBI::dbListTables(con)
+ }
[1] "github_nested"   "github_timeline" "gsod"            "natality"       
[5] "shakespeare"     "trigrams"        "wikipedia"

chilampoon commented 3 years ago

Did your example mean generating the object each time I need it? A reason why I used option() is, most of the time con is not necessary for users, so I wanted to "hide" it

vjcitn commented 3 years ago

It would be generated when needed and discarded when no longer needed. Go ahead with your redesign using mock data and we will discuss further later on. The options() list should not be a place to hide stuff, but we can reevaluate later.

chilampoon commented 3 years ago

Oh there's confusion here, the connection object has to be there for interacting with the database when using this package, so it cannot be discarded all the time. But users don't need to query data by using it directly.

vjcitn commented 3 years ago

Just checking in. Still working on the submission?

chilampoon commented 3 years ago

Just checking in. Still working on the submission?

Yes I'll update the package by next week, sorry for the delay

bioc-issue-bot commented 3 years ago

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

bioc-issue-bot commented 3 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: "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/ReactomeGraph4R to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 3 years ago

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

chilampoon commented 3 years ago

@vjcitn a brief summary:

Since it's unable to connect to the graph database on the builders, I dontrun the examples for all exported functions... and put most of them into a script of manual tests
I added a test for constructing a cypher query, just like your mock example, still some functions need a connection
Saved the data for building the vignette so that no need to directly retrieve them from the database

bioc-issue-bot commented 3 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: "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/ReactomeGraph4R to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 3 years ago

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

bioc-issue-bot commented 3 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: "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/ReactomeGraph4R to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 3 years ago

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

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

chilampoon commented 3 years ago

hi @vjcitn would you review this package? the deadline is coming... thanks

chilampoon commented 3 years ago

Hi @lshep, just wonder if the new package review is still ongoing? The deadline is May 13th right? Thanks.

lshep commented 3 years ago

Yes. Package review process is still going on. Please be patient as reviewers are assigned multiple issue and review in additional to other release tasks. You should have a review shortly.

vjcitn commented 3 years ago

I think we are close here.

 [1] "Update R version dependency from 4.0 to 4.1."                                                                                                                                      
 [2] "License 'Apache License (>= 2.0) | file LICENSE' unknown; licenses cannot restrict use"                                                                                            
 [3] " Avoid redundancy in signalers"                                                                                                                                                    
 [4] "Recommended function length <= 50 lines."                                                                                                                                          
 [5] "Consider adding runnable examples to the following man pages which document exported objects:"                                                                                     
 [6] "Usage of dontrun{} / donttest{} found in man page examples."                                                                                                                       
 [7] "Use donttest{} instead of dontrun{}."

Please address some of these BiocCheck conditions.

bioc-issue-bot commented 3 years ago

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

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

bioc-issue-bot commented 3 years ago

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

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

chilampoon commented 3 years ago

@vjcitn license: Apache License (>= 2.0) | file LICENSE, Apache License (>= 2.0), file LICENSE are all unknown... how should I specify it?

donttest{} would run into an R CMD Check error, so I kept dontrun

bioc-issue-bot commented 3 years ago

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

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

chilampoon commented 3 years ago

There is always a note for the license... @vjcitn what else could be improved? Thanks.

bioc-issue-bot commented 3 years ago

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

Thank you for contributing to Bioconductor!

vjcitn commented 3 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/chilampoon.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("ReactomeGraph4R"). The package 'landing page' will be created at

https://bioconductor.org/packages/ReactomeGraph4R

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

ReactomeGraph4R #1996