v2 cutover

[dogversioning]

Description

This PR cuts over fields to the v2 library field names

Checklist

• Consider if documentation (like in docs/) needs to be updated
• Consider if tests should be added
• Run pylint if you're making changes beyond adding studies

[dogversioning]

in the absence of unit tests, here are some counts from the previous run (before the additional data we added) and the new version:

#   table_name  con
1   covid_symptom__count_dx_month   458
2   covid_symptom__count_dx_week    452
3   covid_symptom__count_pcr_month  0
4   covid_symptom__count_pcr_week   0
5   covid_symptom__count_prevalence_ed_month    0
6   covid_symptom__count_prevalence_ed_week 0
7   covid_symptom__count_study_period_month 11952
8   covid_symptom__count_study_period_week  39720
9   covid_symptom__count_symptom_month  164212
10  covid_symptom__count_symptom_week   271594
11  covid_symptom__define_age   111
12  covid_symptom__define_dx_icd10  1
13  covid_symptom__define_dx_snomed 23
14  covid_symptom__define_pcr   52
15  covid_symptom__define_pcr_custom    4
16  covid_symptom__define_pcr_negative  22
17  covid_symptom__define_pcr_positive  30
18  covid_symptom__define_period    3
19  covid_symptom__define_symptom   155
20  covid_symptom__dx   1734
21  covid_symptom__meta_date    1
22  covid_symptom__meta_version 1
23  covid_symptom__nlp_results  4490835
24  covid_symptom__pcr  0
25  covid_symptom__prevalence_ed    0
26  covid_symptom__site_ed_note 6
27  covid_symptom__study_period 845146
28  covid_symptom__symptom_ctakes_negation  150756

new

#   table_name  con
1   covid_symptom__count_dx_month   584
2   covid_symptom__count_dx_week    526
3   covid_symptom__count_prevalence_ed_month    0
4   covid_symptom__count_prevalence_ed_week 0
5   covid_symptom__count_study_period_month 17050
6   covid_symptom__count_study_period_week  52250
7   covid_symptom__count_symptom_month  164294
8   covid_symptom__count_symptom_week   271716
9   covid_symptom__define_age   111
10  covid_symptom__define_dx_icd10  1
11  covid_symptom__define_dx_snomed 23
12  covid_symptom__define_pcr   52
13  covid_symptom__define_pcr_custom    4
14  covid_symptom__define_pcr_negative  22
15  covid_symptom__define_pcr_positive  30
16  covid_symptom__define_period    3
17  covid_symptom__define_symptom   155
18  covid_symptom__dx   2226
19  covid_symptom__lib_transactions 39
20  covid_symptom__meta_date    1
21  covid_symptom__meta_version 1
22  covid_symptom__nlp_results  4490835
23  covid_symptom__nlp_results_term_exists  3452580
24  covid_symptom__pcr  0
25  covid_symptom__prevalence_ed    0
26  covid_symptom__site_ed_note 6
27  covid_symptom__study_period 2215282
28  covid_symptom__symptom_ctakes_negation  150861
29  covid_symptom__symptom_icd10    20914

We're accepting the no PCR count for now as a defect of our internal implementation and will follow on with a dot release here/in the library to try to address.

[mikix]

🎉 🎊

[mikix]

These new field names are much clearer (if you are familiar with the FHIR spec anyway, and that is probably a given for the audience).

Though... I'm already wondering what you'll do once you decide to abstract away the effectiveX fields - like if core decides to also examine effectiveInstant or effectivePeriod if they are present instead of datetime - do we have a naming convention for that kind of abstraction of multi-format fields?

[dogversioning]

Not yet, no - we're are lightly overloading fields in some places (like period.end, which is often null and we coalesce with period.start).

We can talk about this a bit maybe outside the context of this PR, but I sort of think that's ok as part of the core study's simplification promise - if for some reason it's not, you can roll your own.

[mikix]

Sure - but I want the simplification promise to go even further 😄 - give me just effective_day and that will be the day for whichever of the effectiveX fields happened to be filled out. That's right in core's wheelhouse

[mikix]

Observation.effective especially triggers me because it offers two fields that are so very similar: effectiveDateTime and effectiveInstant - the only difference being whether the field can hold sub-seconds. But now all consumers have to check both fields.

[dogversioning]

I like the effective day promise - like, i'm fine with saying that should always resolve to a reasonable thing.

week and month are trickier, if only because they are very common binning units for studies - we're basically obviating away date casting downstream at the cost of space on disk. I'm sort of ok with this.

[dogversioning]

for posterity - after clarifying some stuff off thread, we created an issue in core to simplify this kind of rat king type handling smart-on-fhir/cumulus-library#205

[mikix]

This is because of the enc_class_code -> enc_class_display field change?

[dogversioning]

Yes, and also that, with a breaking changes in the upstream tables, it felt like a good idea to provide a hard deliniation here.