Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for BIGINT person_id #35

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions .github/workflows/R_CMD_check_Hades.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ jobs:
CDM5_SPARK_CONNECTION_STRING: ${{ secrets.CDM5_SPARK_CONNECTION_STRING }}

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v3

- uses: r-lib/actions/setup-r@v2
with:
Expand Down Expand Up @@ -86,22 +86,37 @@ jobs:

- name: Upload source package
if: success() && runner.os == 'macOS' && github.event_name != 'pull_request' && github.ref == 'refs/heads/main'
uses: actions/upload-artifact@v2
uses: actions/upload-artifact@v4
with:
name: package_tarball
path: check/*.tar.gz

- name: Install covr
if: runner.os == 'macOS'
if: runner.os == 'Linux'
run: |
install.packages("covr")
remotes::install_cran("covr")
remotes::install_cran("xml2")
shell: Rscript {0}

- name: Test coverage
if: runner.os == 'macOS'
run: covr::codecov()
if: runner.os == 'Linux'
run: |
cov <- covr::package_coverage(
quiet = FALSE,
clean = FALSE,
install_path = file.path(normalizePath(Sys.getenv("RUNNER_TEMP"), winslash = "/"), "package")
)
covr::to_cobertura(cov)
shell: Rscript {0}

- uses: codecov/codecov-action@v4
if: runner.os == 'Linux'
with:
file: ./cobertura.xml
plugin: noop
disable_search: true
token: ${{ secrets.CODECOV_TOKEN }}

Release:
needs: R-CMD-Check

Expand Down
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ CohortExplorer 0.1.1
======================
Improve test coverage to 100%
Force creation of .Rproj file for shiny app.
Support BIGINT person IDs.


CohortExplorer 0.1.0
Expand Down
23 changes: 7 additions & 16 deletions R/CreateCohortExplorerApp.R
Original file line number Diff line number Diff line change
Expand Up @@ -172,23 +172,14 @@ createCohortExplorerApp <- function(connectionDetails = NULL,
add = errorMessage
)

if (is.null(personIds)) {
checkmate::assertIntegerish(
x = sampleSize,
lower = 0,
len = 1,
null.ok = TRUE,
# personIds can be interger, numeric, or character to allow bigint (R does not
# support 64-bit integers):
if (!is.null(personIds)) {
checkmate::assertTRUE(
all(grepl("[0-9]+", as.character(personIds))),
add = errorMessage
)
} else {
checkmate::assertIntegerish(
x = personIds,
lower = 0,
min.len = 1,
null.ok = TRUE
)
}

exportFolder <- normalizePath(exportFolder, mustWork = FALSE)

dir.create(
Expand Down Expand Up @@ -331,7 +322,7 @@ createCohortExplorerApp <- function(connectionDetails = NULL,
progressBar = TRUE,
bulkLoad = (Sys.getenv("bulkLoad") == TRUE),
camelCaseToSnakeCase = TRUE,
data = dplyr::tibble(subjectId = as.double(personIds) |> unique())
data = dplyr::tibble(subjectId = as.character(personIds) |> unique())
)

DatabaseConnector::renderTranslateExecuteSql(
Expand All @@ -341,7 +332,7 @@ createCohortExplorerApp <- function(connectionDetails = NULL,
INTO #person_id_data2
FROM #person_id_data a
INNER JOIN #persons_to_filter b
ON a.subject_id = b.subject_id;
ON CAST(a.subject_id AS BIGINT) = CAST(b.subject_id AS BIGINT);

DROP TABLE IF EXISTS #person_id_data;
SELECT DISTINCT subject_id
Expand Down
Loading