API Changes for Incidence Rates

[chrisknoll]

Renaming 'outcomeIds' to 'outcomeCohortIds' to avoid collision with Incidence Rate terminology for 'outcomes'.
Adding incidence rate outcome_id to the output of getIncidenceRates.
Adding tar_id to output of getIncidenceRates
Introduced new template for outcomeCohortIds.

Reason for changes

While historically we referenced outcomes by their cohort IDs, in Incidence rate the Outcome is defined in 2 parts: the cohort that identifies the people, and a clean window which determines a cooldown period between new events.

The CohortIncidence outcome definition can be found here. The outcome_id identifies the cohort_id + clean window parameter. Calling the outcome cohort ID 'outcome id' introduces confusion.

Acknowledging that the OhdsiReportGenerator likes to work in terms of Targets, Outcomes, etc. and fetch results based on those ideas, the API was left in tact to search for results based on a cohort ID, and this remains the case in this PR.

tar_id was added to the output because, like outcome_id, the tar_id uniquely identifies the start/startOffset and end/endOffset. When filtering to a tar (like on-treatment vs intent to treat) you should use tar_ids that identify those (ie: tar_id = 1), not filter on the paramters(ie: start=start, start_offset = 0, end=end, end_offset = 0).

The outcomeCohortIds template was added to correspond to the style in the repository.

[chrisknoll]

My recent commits should address the R check fails as I didn't change the calls in the examples, but I also made the changes more backwards-compatibility friendly:

Initially, the outcomeIds param was renamed to outcomeCohortIds, which is obviously breaking, so to address I left the outcomeIds param in tact, introduced a outcomeCohortIds param which initializes from outcomeIds by default (legacy code will not be passing in outcomeCohortIds, but they will pass in outcomeIds which will be used as the outcomeCohortIds). Internal to the function, we just use outcomeCohortIds because that's what the value actually represents (outcomeId is not an outcomeCohortId in cohort incidence).

[codecov]

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

[jreps]

The outcome id output was named outcomeId for consistency with other results - I do not think it is a good idea to have different names for the same thing across the extraction functions. We should be using the same terms for results extracted from CM, SCCS, PLP, Char and CI.

[chrisknoll]

Thanks for the feedback @jreps. I was thinking the same thing except on the opposite perspective: I do not think it's a good idea to have the same names for different things (outcome_id has a distinct meaning in Cohort Incidence because it encapsulates a cohort_id, clean window and exclusion cohort as an 'outcome', while other aspects only define an outcome with a cohort. Ideally I'd prefer to see the other HADES tools develop a consistent definition of what an outcome is defined as (like it is in CohortIncidence) that goes beyond the current thinking of 'it's just a cohort'.

So, given that we have the term 'outcome_id' meaning one thing in Cohort Incidence and another thing in other aspects, what's the solution to eliminate the confusion when saying 'outcome_id' in cohort incidence? My suggestion would be to give a more accurate name to the things in the other places: outcome_id should be called outcome_cohort_id everywhere, and it's very clear that we are talking about a cohort id for an outcome.

Do you have an alternative?