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

new cohort sql file for v5.3.1. support and a cdm version field in the config file #5

Merged
merged 1 commit into from Mar 26, 2021

Conversation

egillax
Copy link
Contributor

@egillax egillax commented Mar 25, 2021

Hi everyone,

My name is Egill, a postdoc from Erasmus MC in Rotterdam and I was testing your package against some synthetic data. I will eventually also test it on real data.

I needed to make some slight changes to the sql code so it would run on v5.3.1 of the cdm. I think the most common version in use is v5.3.1 so it would be good to support that. The only relevant difference from v5.3.1 to v6 is that the death_dates are not in the person table but in a death table. So I added a new cohort sql file to support that. Then I added a cdm version field in the config.

Then when running the notebooks I just added something like:

if config.CDM_VERSION == 'v5.3.1': 
    cohort_script_path = config.SQL_PATH_COHORTS + '/gen_EOL_cohort_v531.sql'  
else:  
    cohort_script_path = config.SQL_PATH_COHORTS + '/gen_EOL_cohort.sql'  

I didn't include the notebooks in the pull request since then I would overwrite the figures which are there, and since the data I used was synthetic my figures were not so good to have as an example.

Cheers,
Egill

@justinlimkz justinlimkz merged commit e152dbf into clinicalml:master Mar 26, 2021
@justinlimkz
Copy link
Contributor

Hi Egill, thank you very much for testing out the package and adding v5.3.1 compatibility! I'm merging your pull request, and will edit the notebook to include the config.CDM_VERSION check you mentioned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants