Skip to content

Commit

Permalink
feat: improve variable name to reflect its real nature
Browse files Browse the repository at this point in the history
...which is cohort_definition_id...there is no such thing as "cohort id"
  • Loading branch information
pieterlukasse committed Dec 14, 2023
1 parent e783f33 commit faed7c2
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 91 deletions.
6 changes: 3 additions & 3 deletions controllers/cohortdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func generateCohortPairsHeaders(cohortPairs []utils.CustomDichotomousVariableDef
cohortPairsHeaders := []string{}

for _, cohortPair := range cohortPairs {
cohortPairsHeaders = append(cohortPairsHeaders, utils.GetCohortPairKey(cohortPair.CohortId1, cohortPair.CohortId2))
cohortPairsHeaders = append(cohortPairsHeaders, utils.GetCohortPairKey(cohortPair.CohortDefinitionId1, cohortPair.CohortDefinitionId2))
}

return cohortPairsHeaders
Expand Down Expand Up @@ -298,8 +298,8 @@ func (u CohortDataController) RetrievePeopleIdAndCohort(sourceId int, cohortId i
*/
personIdToCSVValues := make(map[int64]map[string]string)
for _, cohortPair := range cohortPairs {
firstCohortDefinitionId := cohortPair.CohortId1
secondCohortDefinitionId := cohortPair.CohortId2
firstCohortDefinitionId := cohortPair.CohortDefinitionId1
secondCohortDefinitionId := cohortPair.CohortDefinitionId2
cohortPairKey := utils.GetCohortPairKey(firstCohortDefinitionId, secondCohortDefinitionId)

firstCohortPeopleData, err1 := u.cohortDataModel.RetrieveDataByOriginalCohortAndNewCohort(sourceId, cohortId, firstCohortDefinitionId)
Expand Down
4 changes: 2 additions & 2 deletions models/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func QueryFilterByCohortPairsHelper(filterCohortPairs []utils.CustomDichotomousV
"UNION " +
"SELECT subject_id FROM " + resultsDataSource.Schema + ".cohort WHERE cohort_definition_id=? " +
")"
idsList = append(idsList, filterCohortPair.CohortId1, filterCohortPair.CohortId2)
idsList = append(idsList, filterCohortPair.CohortDefinitionId1, filterCohortPair.CohortDefinitionId2)
}
// EXCEPTs section:
for _, filterCohortPair := range filterCohortPairs {
Expand All @@ -54,7 +54,7 @@ func QueryFilterByCohortPairsHelper(filterCohortPairs []utils.CustomDichotomousV
"INTERSECT " +
"SELECT subject_id FROM " + resultsDataSource.Schema + ".cohort WHERE cohort_definition_id=? " +
")"
idsList = append(idsList, filterCohortPair.CohortId1, filterCohortPair.CohortId2)
idsList = append(idsList, filterCohortPair.CohortDefinitionId1, filterCohortPair.CohortDefinitionId2)
}
}
unionAndIntersectSQL = unionAndIntersectSQL +
Expand Down
36 changes: 18 additions & 18 deletions tests/controllers_tests/controllers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -717,14 +717,14 @@ func TestGetAttritionRowForConceptIdsAndCohortPairs(t *testing.T) {
int64(1234),
int64(5678),
utils.CustomDichotomousVariableDef{
CohortId1: 1,
CohortId2: 2,
ProvidedName: "testA12"},
CohortDefinitionId1: 1,
CohortDefinitionId2: 2,
ProvidedName: "testA12"},
int64(2090006880),
utils.CustomDichotomousVariableDef{
CohortId1: 3,
CohortId2: 4,
ProvidedName: "testB34"},
CohortDefinitionId1: 3,
CohortDefinitionId2: 4,
ProvidedName: "testB34"},
}

result, _ := conceptController.GetAttritionRowForConceptIdsAndCohortPairs(sourceId, cohortId, conceptIdsAndCohortPairs, breakdownConceptId, sortedConceptValues)
Expand Down Expand Up @@ -771,9 +771,9 @@ func TestGenerateCompleteCSV(t *testing.T) {

cohortPairs := []utils.CustomDichotomousVariableDef{
{
CohortId1: 2,
CohortId2: 3,
ProvidedName: "test"},
CohortDefinitionId1: 2,
CohortDefinitionId2: 3,
ProvidedName: "test"},
}

b := controllers.GenerateCompleteCSV(partialCsv, personIdToCSVValues, cohortPairs)
Expand All @@ -798,9 +798,9 @@ func TestRetrievePeopleIdAndCohort(t *testing.T) {
cohortId := 1
cohortPairs := []utils.CustomDichotomousVariableDef{
{
CohortId1: 2,
CohortId2: 3,
ProvidedName: "test"},
CohortDefinitionId1: 2,
CohortDefinitionId2: 3,
ProvidedName: "test"},
}

cohortData := []*models.PersonConceptAndValue{
Expand Down Expand Up @@ -839,9 +839,9 @@ func TestRetrievePeopleIdAndCohortNonExistingCohortPair(t *testing.T) {
cohortId := 1
cohortPairs := []utils.CustomDichotomousVariableDef{
{
CohortId1: 4,
CohortId2: 5,
ProvidedName: "test"},
CohortDefinitionId1: 4,
CohortDefinitionId2: 5,
ProvidedName: "test"},
}

cohortData := []*models.PersonConceptAndValue{
Expand Down Expand Up @@ -880,9 +880,9 @@ func TestRetrievePeopleIdAndCohortOverlappingCohortPair(t *testing.T) {
cohortId := 1
cohortPairs := []utils.CustomDichotomousVariableDef{
{
CohortId1: 1,
CohortId2: 1,
ProvidedName: "test"},
CohortDefinitionId1: 1,
CohortDefinitionId2: 1,
ProvidedName: "test"},
}

cohortData := []*models.PersonConceptAndValue{
Expand Down
116 changes: 58 additions & 58 deletions tests/models_tests/models_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,9 @@ func TestQueryFilterByCohortPairsHelper(t *testing.T) {
// smallestCohort and largestCohort do not overlap...
filterCohortPairs := []utils.CustomDichotomousVariableDef{
{
CohortId1: smallestCohort.Id,
CohortId2: largestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: smallestCohort.Id,
CohortDefinitionId2: largestCohort.Id,
ProvidedName: "test"},
}
resultsDataSource := tests.GetResultsDataSource()
var subjectIds []*SubjectId
Expand All @@ -267,13 +267,13 @@ func TestQueryFilterByCohortPairsHelper(t *testing.T) {
// now add a pair that overlaps with largestCohort:
filterCohortPairs = []utils.CustomDichotomousVariableDef{
{
CohortId1: smallestCohort.Id,
CohortId2: largestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: smallestCohort.Id,
CohortDefinitionId2: largestCohort.Id,
ProvidedName: "test"},
{
CohortId1: extendedCopyOfSecondLargestCohort.Id,
CohortId2: largestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: extendedCopyOfSecondLargestCohort.Id,
CohortDefinitionId2: largestCohort.Id,
ProvidedName: "test"},
}
subjectIds = []*SubjectId{}
population = largestCohort
Expand All @@ -289,13 +289,13 @@ func TestQueryFilterByCohortPairsHelper(t *testing.T) {
// order doesn't matter:
filterCohortPairs = []utils.CustomDichotomousVariableDef{
{
CohortId1: extendedCopyOfSecondLargestCohort.Id,
CohortId2: largestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: extendedCopyOfSecondLargestCohort.Id,
CohortDefinitionId2: largestCohort.Id,
ProvidedName: "test"},
{
CohortId1: smallestCohort.Id,
CohortId2: largestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: smallestCohort.Id,
CohortDefinitionId2: largestCohort.Id,
ProvidedName: "test"},
}
subjectIds = []*SubjectId{}
population = largestCohort
Expand All @@ -311,9 +311,9 @@ func TestQueryFilterByCohortPairsHelper(t *testing.T) {
// now test with two other cohorts that overlap:
filterCohortPairs = []utils.CustomDichotomousVariableDef{
{
CohortId1: secondLargestCohort.Id,
CohortId2: extendedCopyOfSecondLargestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: secondLargestCohort.Id,
CohortDefinitionId2: extendedCopyOfSecondLargestCohort.Id,
ProvidedName: "test"},
}
subjectIds = []*SubjectId{}
population = extendedCopyOfSecondLargestCohort
Expand All @@ -329,13 +329,13 @@ func TestQueryFilterByCohortPairsHelper(t *testing.T) {
// now add in the largestCohort as a pair of extendedCopyOfSecondLargestCohort to the mix above:
filterCohortPairs = []utils.CustomDichotomousVariableDef{
{
CohortId1: secondLargestCohort.Id,
CohortId2: extendedCopyOfSecondLargestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: secondLargestCohort.Id,
CohortDefinitionId2: extendedCopyOfSecondLargestCohort.Id,
ProvidedName: "test"},
{
CohortId1: largestCohort.Id,
CohortId2: extendedCopyOfSecondLargestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: largestCohort.Id,
CohortDefinitionId2: extendedCopyOfSecondLargestCohort.Id,
ProvidedName: "test"},
}
subjectIds = []*SubjectId{}
population = extendedCopyOfSecondLargestCohort
Expand Down Expand Up @@ -376,27 +376,27 @@ func TestQueryFilterByCohortPairsHelper(t *testing.T) {
// should return 0:
filterCohortPairs = []utils.CustomDichotomousVariableDef{
{
CohortId1: largestCohort.Id,
CohortId2: largestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: largestCohort.Id,
CohortDefinitionId2: largestCohort.Id,
ProvidedName: "test"},
}
subjectIds = []*SubjectId{}
population = largestCohort
resultsDataSource = tests.GetResultsDataSource()
query = models.QueryFilterByCohortPairsHelper(filterCohortPairs, resultsDataSource, population.Id, "unionAndIntersect").
Select("subject_id")
_ = query.Scan(&subjectIds)
// in this case we expect overlap the size to be 0, since the pair is composed of the same cohort in CohortId1 and CohortId2 and their overlap is excluded:
// in this case we expect overlap the size to be 0, since the pair is composed of the same cohort in CohortDefinitionId1 and CohortDefinitionId2 and their overlap is excluded:
if len(subjectIds) != 0 {
t.Errorf("Expected 0 overlap, found %d", len(subjectIds))
}

// should return 0:
filterCohortPairs = []utils.CustomDichotomousVariableDef{
{
CohortId1: thirdLargestCohort.Id,
CohortId2: largestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: thirdLargestCohort.Id,
CohortDefinitionId2: largestCohort.Id,
ProvidedName: "test"},
}
subjectIds = []*SubjectId{}
population = smallestCohort
Expand All @@ -417,9 +417,9 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndTwoCohortPai
// setting the largest and smallest cohorts here as a pair:
filterCohortPairs := []utils.CustomDichotomousVariableDef{
{
CohortId1: smallestCohort.Id,
CohortId2: largestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: smallestCohort.Id,
CohortDefinitionId2: largestCohort.Id,
ProvidedName: "test"},
}
breakdownConceptId := hareConceptId // not normally the case...but we'll use the same here just for the test...
stats, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId,
Expand All @@ -439,13 +439,13 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndTwoCohortPai
// and because of an overlaping person found in the two cohorts of the new pair.
filterCohortPairs = []utils.CustomDichotomousVariableDef{
{
CohortId1: smallestCohort.Id,
CohortId2: largestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: smallestCohort.Id,
CohortDefinitionId2: largestCohort.Id,
ProvidedName: "test"},
{
CohortId1: secondLargestCohort.Id,
CohortId2: extendedCopyOfSecondLargestCohort.Id,
ProvidedName: "test2"},
CohortDefinitionId1: secondLargestCohort.Id,
CohortDefinitionId2: extendedCopyOfSecondLargestCohort.Id,
ProvidedName: "test2"},
}
stats, _ = conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId,
populationCohort.Id, filterIds, filterCohortPairs, breakdownConceptId)
Expand All @@ -464,9 +464,9 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairsW
// setting the same cohort id here (artificial...but just to check if that returns the same value as when this filter is not there):
filterCohortPairs := []utils.CustomDichotomousVariableDef{
{
CohortId1: secondLargestCohort.Id,
CohortId2: extendedCopyOfSecondLargestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: secondLargestCohort.Id,
CohortDefinitionId2: extendedCopyOfSecondLargestCohort.Id,
ProvidedName: "test"},
}
breakdownConceptId := hareConceptId // not normally the case...but we'll use the same here just for the test...
stats, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId,
Expand Down Expand Up @@ -499,9 +499,9 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairsW
// setting the same cohort id here (artificial...normally it should be two different ids):
filterCohortPairs = []utils.CustomDichotomousVariableDef{
{
CohortId1: smallestCohort.Id,
CohortId2: largestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: smallestCohort.Id,
CohortDefinitionId2: largestCohort.Id,
ProvidedName: "test"},
}
stats3, _ := conceptModel.RetrieveBreakdownStatsBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId,
secondLargestCohort.Id, filterIds, filterCohortPairs, breakdownConceptId)
Expand Down Expand Up @@ -569,9 +569,9 @@ func TestGetTeamProjectsThatMatchAllCohortDefinitionIdsOnlyDefaultMatch(t *testi
cohortDefinitionId := 2
filterCohortPairs := []utils.CustomDichotomousVariableDef{
{
CohortId1: smallestCohort.Id,
CohortId2: largestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: smallestCohort.Id,
CohortDefinitionId2: largestCohort.Id,
ProvidedName: "test"},
}
uniqueCohortDefinitionIdsList := utils.GetUniqueCohortDefinitionIdsListFromRequest(cohortDefinitionId, filterCohortPairs)
teamProjects, _ := cohortDefinitionModel.GetTeamProjectsThatMatchAllCohortDefinitionIds(uniqueCohortDefinitionIdsList)
Expand All @@ -585,9 +585,9 @@ func TestGetTeamProjectsThatMatchAllCohortDefinitionIds(t *testing.T) {
cohortDefinitionId := 2
filterCohortPairs := []utils.CustomDichotomousVariableDef{
{
CohortId1: 2,
CohortId2: 2,
ProvidedName: "test"},
CohortDefinitionId1: 2,
CohortDefinitionId2: 2,
ProvidedName: "test"},
}
uniqueCohortDefinitionIdsList := utils.GetUniqueCohortDefinitionIdsListFromRequest(cohortDefinitionId, filterCohortPairs)
teamProjects, _ := cohortDefinitionModel.GetTeamProjectsThatMatchAllCohortDefinitionIds(uniqueCohortDefinitionIdsList)
Expand Down Expand Up @@ -710,9 +710,9 @@ func TestRetrieveHistogramDataBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(t
// now filter on the extendedCopyOfSecondLargestCohort
filterCohortPairs = []utils.CustomDichotomousVariableDef{
{
CohortId1: smallestCohort.Id,
CohortId2: extendedCopyOfSecondLargestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: smallestCohort.Id,
CohortDefinitionId2: extendedCopyOfSecondLargestCohort.Id,
ProvidedName: "test"},
}
// then we expect histogram data for the overlapping population only (which is 5 for extendedCopyOfSecondLargestCohort and largestCohort):
data, _ = cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(testSourceId, largestCohort.Id, histogramConceptId, filterConceptIds, filterCohortPairs)
Expand Down Expand Up @@ -840,9 +840,9 @@ func TestRetrieveCohortOverlapStatsWithoutFilteringOnConceptValue(t *testing.T)
controlCohortId = largestCohort.Id // to ensure we get largestCohort as initial overlap, just repeat the same here...
filterCohortPairs = []utils.CustomDichotomousVariableDef{
{
CohortId1: smallestCohort.Id,
CohortId2: extendedCopyOfSecondLargestCohort.Id,
ProvidedName: "test"},
CohortDefinitionId1: smallestCohort.Id,
CohortDefinitionId2: extendedCopyOfSecondLargestCohort.Id,
ProvidedName: "test"},
}
// then we expect overlap of 5 for extendedCopyOfSecondLargestCohort and largestCohort:
stats, _ = cohortDataModel.RetrieveCohortOverlapStatsWithoutFilteringOnConceptValue(testSourceId, caseCohortId, controlCohortId,
Expand Down
6 changes: 3 additions & 3 deletions tests/utils_tests/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ func TestParsePrefixedConceptIdsAndDichotomousIds(t *testing.T) {

expectedCohortPairs := []utils.CustomDichotomousVariableDef{
{
CohortId1: 1,
CohortId2: 3,
ProvidedName: "test"},
CohortDefinitionId1: 1,
CohortDefinitionId2: 3,
ProvidedName: "test"},
}

for i, cohortPair := range cohortPairs {
Expand Down
14 changes: 7 additions & 7 deletions utils/parsing.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,9 @@ type ConceptTypes struct {

// fields that define a custom dichotomous variable:
type CustomDichotomousVariableDef struct {
CohortId1 int
CohortId2 int
ProvidedName string
CohortDefinitionId1 int
CohortDefinitionId2 int
ProvidedName string
}

func GetCohortPairKey(firstCohortDefinitionId int, secondCohortDefinitionId int) string {
Expand Down Expand Up @@ -144,9 +144,9 @@ func ParseConceptIdsAndDichotomousDefsAsSingleList(c *gin.Context) ([]interface{
providedName = variable["provided_name"].(string)
}
customDichotomousVariableDef := CustomDichotomousVariableDef{
CohortId1: cohortPair[0],
CohortId2: cohortPair[1],
ProvidedName: providedName,
CohortDefinitionId1: cohortPair[0],
CohortDefinitionId2: cohortPair[1],
ProvidedName: providedName,
}
conceptIdsAndCohortPairs = append(conceptIdsAndCohortPairs, customDichotomousVariableDef)
}
Expand Down Expand Up @@ -294,7 +294,7 @@ func GetUniqueCohortDefinitionIdsListFromRequest(cohortDefinitionId int, filterC
idsList = append(idsList, cohortDefinitionId)
if len(filterCohortPairs) > 0 {
for _, filterCohortPair := range filterCohortPairs {
idsList = append(idsList, filterCohortPair.CohortId1, filterCohortPair.CohortId2) // TODO - rename CohortId1/2 here to cohortDefinition1/2
idsList = append(idsList, filterCohortPair.CohortDefinitionId1, filterCohortPair.CohortDefinitionId2)
}
}
uniqueIdsList := MakeUnique(idsList)
Expand Down

0 comments on commit faed7c2

Please sign in to comment.