vjcitn commented 1 year ago

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

Repository: https://github.com/vjcitn/BiocFHIR

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

Hi @vjcitn

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: BiocFHIR
Title: Illustration of FHIR ingestion and transformation using R
Version: 0.99.0
Date: 2022-10-06
Authors@R: 
    c(person(
        "Vincent", "Carey", role = c("aut", "cre"),
        email = "stvjc@channing.harvard.edu"
    ))
Description: FHIR R4 bundles in JSON format are derived from https://synthea.mitre.org/downloads.
    Transformation inspired by a kaggle notebook published by Dr
    Alexander Scarlat, https://www.kaggle.com/code/drscarlat/fhir-starter-parse-healthcare-bundles-into-tables.
License: Artistic-2.0
Encoding: UTF-8
Depends: R (>= 4.2)
Imports:
    DT,
    shiny,
    jsonlite, graph,
    tidyr, visNetwork, igraph, utils, methods
Suggests: knitr, testthat
LazyData: true
VignetteBuilder: knitr
biocViews: Infrastructure
RoxygenNote: 7.2.1
URL: https://github.com/vjcitn/BiocFHIR
BugReports: https://github.com/vjcitn/BiocFHIR/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". 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/BiocFHIR 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: 9685e4e7a7d243d9fbe4aabc2c26833e026ee9ae

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/BiocFHIR 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: 978097fae192119c192ba4c9ec7e35e233538d26

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/BiocFHIR 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: 4a79ae1ad4704d0b0fc0a05e53a02aaf73b57afb

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

LiNk-NY commented 1 year ago

Hi Vince, @vjcitn

Thank you for your submission. Please see the review below.

Best regards, Marcel

BiocFHIR #2836

This package is highly valuable for importing FHIR bundles.
I wonder if working with nested JSON data could benefit from rjsoncons::jmespath operations. Are there consistent structures in the JSON files to merit the use of rjsoncons?

DESCRIPTION

Looks good.

NAMESPACE

Looks good.

vignettes/

Add an Installation section to the main vignette.
Consider using the following chunk options for the vignettes when loading packages with library: {r,include=TRUE,results="hide",message=FALSE,warning=FALSE}

I'm getting an error when running the following from the D_linking.Rmd vignette:

> g = make_condition_graph(allin)
Error in process_Condition(x$Condition) : 
inherits(Condition, "BiocFHIR.Condition") is not TRUE
some bundles had no Condition component

as well as:

> g = add_procedures(g, allin)
Error in process_Procedure(x$Procedure) : 
inherits(Procedure, "BiocFHIR.Procedure") is not TRUE
some bundles had no Procedure component

though it seems that g is created despite the error.

R

See the error:

>  FHIRtabs()
Error in vapply(zns, "[", 1, character(1)) : 
values must be type 'double',
but FUN(X[[1]]) result is type 'character'

This means that arguments in both vapply calls (in app.R) are in incorrect order:

fn <- vapply(zns, "[", 1, character(1))
ln <- vapply(zns, "[", 2, character(1))

compared to

fn <- vapply(zns, "[", character(1), 1)
ln <- vapply(zns, "[", character(1), 2)

Avoid changing the working directory and use the exdir argument in unzip for make_test_json_set
Avoid using both <- and -> in process_CarePlan
Are files cleared for public use? Should names be anonymized?
Can the ResourceTypes and retention_schemas be sourced from some specification file? How will schemas.R be updated when these fields change?
Minor. Consider using BiocBaseUtils::selectSome.

tests

Increase package coverage:

> covr::package_coverage()
BiocFHIR Coverage: 13.44%
R/app.R: 0.00%
R/graphit.R: 0.00%
R/make_test_json_set.R: 0.00%
R/process_CarePlan.R: 0.00%
R/process_Claim.R: 0.00%
R/process_Condition.R: 0.00%
R/process_Observation.R: 0.00%
R/processOthers.R: 0.00%
R/process_Patient.R: 0.00%
R/schemas.R: 75.00%
R/use_json.R: 87.50%

/

Remove the docs folder

bioc-issue-bot commented 1 year ago

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

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/BiocFHIR 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: 4666297ff7abb6a69f9ae2a4ddbaee01ada677bb

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/BiocFHIR 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: e4969e76a0774d4bef4c5d0a10cd6be3773b3598

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

vjcitn commented 1 year ago

Changes in 0.99.7

vignettes/
Add an Installation section to the main vignette.

Done

Consider using the following chunk options for the vignettes when loading
packages with library:
{r,include=TRUE,results="hide",message=FALSE,warning=FALSE}

Missed this one, will learn the pattern.

I'm getting an error when running the following from the D_linking.Rmd vignette:
> g = make_condition_graph(allin)
Error in process_Condition(x$Condition) : 
  inherits(Condition, "BiocFHIR.Condition") is not TRUE
some bundles had no Condition component
as well as:

> g = add_procedures(g, allin)
Error in process_Procedure(x$Procedure) : 
  inherits(Procedure, "BiocFHIR.Procedure") is not TRUE
some bundles had no Procedure component
though it seems that g is created despite the error.

Yes, that's right -- those errors are caught by try. It is a serious issue in the FHIR documents when a component like Procedure is not available, and I don't want it to be ignored. I could change to message.

R
See the error:
>  FHIRtabs()
Error in vapply(zns, "[", 1, character(1)) : 
  values must be type 'double',
 but FUN(X[[1]]) result is type 'character'
This means that arguments in both vapply calls (in app.R) are in incorrect
order:

fn <- vapply(zns, "[", 1, character(1))
ln <- vapply(zns, "[", 2, character(1))
compared to

fn <- vapply(zns, "[", character(1), 1)
ln <- vapply(zns, "[", character(1), 2)

That's fixed

Avoid changing the working directory and use the exdir argument in unzip
for make_test_json_set

Done

Avoid using both <- and -> in process_CarePlan

Done

Are files cleared for public use? Should names be anonymized?

The names are fake. See Synthea site

Can the ResourceTypes and retention_schemas be sourced from some
specification file? How will schemas.R be updated when these fields change?

I agree with this concern. This set of exercises/schemas may have to be renamed "Synthea-MITRE" because they are adapted specifically to this way of organizing FHIR resources; different hierarchies could be used in different FHIR resources. In the long run FHIR services/servers will be interrogated directly, but I don't know of a public server to illustrate this meaningfully at this moment.

Minor. Consider using BiocBaseUtils::selectSome.

Done.

tests
Increase package coverage:
> covr::package_coverage()
BiocFHIR Coverage: 13.44%
R/app.R: 0.00%
R/graphit.R: 0.00%
R/make_test_json_set.R: 0.00%
R/process_CarePlan.R: 0.00%
R/process_Claim.R: 0.00%
R/process_Condition.R: 0.00%
R/process_Observation.R: 0.00%
R/processOthers.R: 0.00%
R/process_Patient.R: 0.00%
R/schemas.R: 75.00%
R/use_json.R: 87.50%

When I use type="all" I get much better results with package_coverage. I agree this must be done in a unit testing framework, but for now the examples cover, and will throw errors in CMD check if there are problems.

/
Remove the docs folder

Done. branch with_pkgdown created.

vjcitn commented 1 year ago

BTW I did add illustration of rjsoncons in the vignette Bhandling.Rmd. Ultimately the process* functions may be based on JMESPATH queries, and the "schemas" could be reformulated as JMESPATH patterns.

LiNk-NY commented 1 year ago

BTW I did add illustration of rjsoncons in the vignette Bhandling.Rmd. Ultimately the process* functions may be based on JMESPATH queries, and the "schemas" could be reformulated as JMESPATH patterns.

@vjcitn This looks good. The query can be made a bit simpler with:

tmp = jmespath(myl,
  "[*].entry[0].resource.address.[*].[city,state,postalCode,country][]") |>
  jsonlite::fromJSON()
dimnames(tmp) <- list(NULL, c("city", "state", "postalCode", "country"))
as.data.frame(tmp)
#'          city         state postalCode country
#' 1      Malden Massachusetts      02148      US
#' 2 Springfield Massachusetts      01013      US
#' 3    Brewster Massachusetts      02631      US
#' 4     Webster Massachusetts      01570      US

LiNk-NY commented 1 year ago

I'm getting an error when running the following from the D_linking.Rmd vignette:
> g = make_condition_graph(allin)
Error in process_Condition(x$Condition) : 
  inherits(Condition, "BiocFHIR.Condition") is not TRUE
some bundles had no Condition component
as well as:

> g = add_procedures(g, allin)
Error in process_Procedure(x$Procedure) : 
  inherits(Procedure, "BiocFHIR.Procedure") is not TRUE
some bundles had no Procedure component
though it seems that g is created despite the error.
Yes, that's right -- those errors are caught by try. It is a serious issue in the FHIR documents when a component like Procedure is not available, and I don't want it to be ignored. I could change to message.

I think this could be made into a warning? Everything else looks good. Thanks!

LiNk-NY commented 1 year ago

Note. Here are a couple diffs that would resolve notes / warnings in the package.

R/app.R

diff --git a/R/app.R b/R/app.R
index e49b74d..13ddf15 100644
--- a/R/app.R
+++ b/R/app.R
@@ -112,7 +112,9 @@ an individual.  A 'fallback' schema usage may be adopted in future versions to c
          })
      output$network <- visNetwork::renderVisNetwork({
          allin <- NULL
-         data("allin", package="BiocFHIR")
+         data_store <- new.env(parent = emptyenv())
+         data("allin", package="BiocFHIR", envir = data_store)
+         allin <- data_store[["allin"]]
          BiocFHIR::FHIR_retention_schemas()
          g <- build_proccond_igraph( allin ) 
          display_proccond_igraph( g )

R/data.R

diff --git a/R/data.R b/R/data.R
index f888141..4361d86 100644
--- a/R/data.R
+++ b/R/data.R
@@ -4,4 +4,5 @@
 #' data("allin", package="BiocFHIR")
 #' allin[[1]]
 #' @format list
+#' @usage data("allin", package = "BiocFHIR")
 "allin"

bioc-issue-bot commented 1 year ago

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

vjcitn commented 1 year ago

These have all been addressed. The make_condition_graph now includes a parameter to filter away bundles that lack "Condition", so the error is not triggered in the vignette. Other suggestions were adopted and a NEWS.Rd file was added.

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/BiocFHIR 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: b783b8eebf2f871e37c6180d47e9415afe90c14e

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/BiocFHIR 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: 852ca5f8c11e600efcbb1490ef7726e42849db49

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

LiNk-NY commented 1 year ago

Hi Vince, @vjcitn Thanks for making those changes. The package looks good to go. Best regards, Marcel

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

https://bioconductor.org/packages/BiocFHIR

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

BiocFHIR #2836

BiocFHIR #2836

DESCRIPTION

NAMESPACE

vignettes/

R

tests

/

R/app.R

R/data.R