OHDSI / Eunomia

An R package that facilitates access to a variety of OMOP CDM sample data sets.
https://ohdsi.github.io/Eunomia/
42 stars 11 forks source link

Update GiBleed.sql #24

Closed priagopal closed 2 years ago

priagopal commented 2 years ago

Hi,

I have updated the code. Can you please take a look.

Thanks, priya

ablack3 commented 2 years ago

Hi @priagopal,

Thanks for this PR. I have a few comments.

First you will need to work off of the "develop" branch and then make your PR on the OHDSI/develop branch. So in your local clone checkout the develop branch, make sure it matches the upstream OHDSI develop branch, make your commits, and then create a PR to merge those changes into the develop branch. The reason is that "main" is our current release so the only commits that get merged into "main" are new releases.

Regarding the SQL I think it should be

INSERT INTO @cohort_database_schema.@cohort_table (
    cohort_definition_id, 
    subject_id, 
    cohort_start_date, 
    cohort_end_date
    )
SELECT CAST(@cohort_definition_id AS INT) AS cohort_definition_id,
    condition_occurrence.person_id,
    condition_start_date,
    COALESCE(condition_end_date, condition_start_date)
FROM @cdm_database_schema.condition_occurrence
WHERE condition_concept_id IN (
        SELECT descendant_concept_id
        FROM @cdm_database_schema.concept_ancestor
        WHERE ancestor_concept_id = 192671 -- Gastrointestinal haemorrhage
        );

COALESCE(condition_end_date, condition_start_date) should always give us an end date. I think your change to the SQL will result in a cohort with 0 rows.

library(Eunomia)
#> Loading required package: DatabaseConnector
cd <- getEunomiaConnectionDetails()
con <- connect(cd)
#> Connecting using SQLite driver
tibble::tibble(dbGetQuery(con, "
SELECT CAST(1 AS INT) AS cohort_definition_id,
    condition_occurrence.person_id,
    condition_start_date,
    condition_end_date
FROM main.condition_occurrence
WHERE condition_end_date IS NOT NULL AND condition_concept_id IN (
        SELECT descendant_concept_id
        FROM main.concept_ancestor
        WHERE ancestor_concept_id = 192671 -- Gastrointestinal haemorrhage
        )"))
#> # A tibble: 0 × 4
#> # … with 4 variables: cohort_definition_id <lgl>, PERSON_ID <dbl>,
#> #   CONDITION_START_DATE <dbl>, CONDITION_END_DATE <dbl>
#> # ℹ Use `colnames()` to see all variable names

Created on 2022-08-04 by the reprex package (v2.0.1)

library(Eunomia)
#> Loading required package: DatabaseConnector
cd <- getEunomiaConnectionDetails()
con <- connect(cd)
#> Connecting using SQLite driver
tibble::tibble(dbGetQuery(con, "
SELECT CAST(1 AS INT) AS cohort_definition_id,
    condition_occurrence.person_id,
    condition_start_date,
    COALESCE(condition_end_date, condition_start_date)
FROM main.condition_occurrence
WHERE condition_concept_id IN (
        SELECT descendant_concept_id
        FROM main.concept_ancestor
        WHERE ancestor_concept_id = 192671 -- Gastrointestinal haemorrhage
        )"))
#> # A tibble: 479 × 4
#>    cohort_definition_id PERSON_ID CONDITION_START_DATE COALESCE(condition_end_…¹
#>                   <dbl>     <dbl>                <dbl>                     <dbl>
#>  1                    1       273           1318204800                1318204800
#>  2                    1        61           1126742400                1126742400
#>  3                    1       351           1530144000                1530144000
#>  4                    1       579            941846400                 941846400
#>  5                    1       549            567648000                 567648000
#>  6                    1       116              6048000                   6048000
#>  7                    1       163           1272153600                1272153600
#>  8                    1       304            894153600                 894153600
#>  9                    1       326           1081555200                1081555200
#> 10                    1       285           1155686400                1155686400
#> # … with 469 more rows, and abbreviated variable name
#> #   ¹​`COALESCE(condition_end_date, condition_start_date)`
#> # ℹ Use `print(n = ...)` to see more rows

Created on 2022-08-04 by the reprex package (v2.0.1)

ablack3 commented 2 years ago

@fdefalco, I'm a little confused why the develop branch is behind main. Shouldn't "develop" already have all commits that "main" has?

fdefalco commented 2 years ago

It appears that there are 4 commits that were made directly to main. I believe part of the issue arose when we submitted to CRAN but then never received resolution. Could we attempt to update develop from main and merge conflicts to get back to proper order?

ablack3 commented 2 years ago

Looks good. Thanks @priagopal!

priagopal commented 2 years ago

Thanks to Adam and Frank. I really appreciate your help.