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

feat: interface for comparer and builder #198

Open
wants to merge 12 commits into
base: epic/157-create-an-evaluatorcomparer
Choose a base branch
from

Conversation

LauraBoenchenLB
Copy link
Contributor

@LauraBoenchenLB LauraBoenchenLB commented Sep 11, 2024

Adds Interface for DataComparer and the corresponding Builder.

DataComparer will be used to evaluate differences between multiple MedRecords or subcohorts of MedRecords.

Added a Class for creating the Cohort Config. All the functions in the DataComparer need one or multiple configured Cohorts as input. All outputs are typed dictionaries to be able to access the values easier for the user in case they want to plot them. They will also be displayed as tables similar to the edge/node overview table.

The hypothesis tests will be selected based on the type and distribution of the MedRecordAttributes and may not be possible for all attributes.

Examples:

case_group = CohortEvaluator(
    medrecord=medrecord,
    name="control",
    cohort_group="patients",
    attributes={"age": "Age", "gender": "Gender"},
    concepts_groups={
        "diagnoses": "diagnoses",
        "medications": "medications",
        "procedures": "procedures",
    },
)

control_group = CohortEvaluator(
    medrecord=medrecord2,
    name="control",
    cohort_group="patients",
    attributes={"age": "age", "gender": "gender"},
    concepts_groups={
        "diagnoses": "Diagnoses",
        "medications": "Medications",
        "procedures": "Procedures",
    },
)

# evaluating single group
control_group.get_concept_counts()

CohortComparer.calculate_distance_concepts(case_group, control_group)

# compare between groups
CohortComparer.test_difference_cohort_attributes(
    cohorts_attribute=[case_group, control_group],
    significance_level=0.05
)

# Output
{
    "age": [
        {
            "test": "t_test",
            "Hypothesis": "There is no significant difference between the means of the two populations.",
            "not_reject": True,
            "p_value": 0.1,
        }
    ],
    "income": [
        {
            "test": "t_test",
            "Hypothesis": "There is no significant difference between the means of the two populations.",
            "not_reject": False,
            "p_value": 0.6,
        }
    ],
}


@LauraBoenchenLB LauraBoenchenLB force-pushed the 192-define-evaluatorcomparer-interface branch from b3b4afc to 3e202d9 Compare September 11, 2024 15:19
Copy link
Contributor

@MarIniOnz MarIniOnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start to get our comparer!

I have a couple of overall observations.

  • It would be nice to have some overall comparison tables, like number of nodes of each subcohort, number of edges between those nodes, number of different attributes... Something like what happened in the print_table attributes and so. I do not know if this the idea behind the "Full_report"

  • Do we want to have a comparer that can take two medrecords or two subcohorts or separate them? Could be better to have separated so it is not so confusing. That is a question that can be debated in the MedModels Weekly.

medmodels/statistic_evaluations/comparer/builder.pyi Outdated Show resolved Hide resolved
medmodels/statistic_evaluations/comparer/builder.pyi Outdated Show resolved Hide resolved
medmodels/statistic_evaluations/comparer/builder.pyi Outdated Show resolved Hide resolved
medmodels/statistic_evaluations/comparer/data_comparer.pyi Outdated Show resolved Hide resolved
medmodels/statistic_evaluations/comparer/data_comparer.pyi Outdated Show resolved Hide resolved
Copy link
Contributor

@MarIniOnz MarIniOnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More comments

@LauraBoenchenLB LauraBoenchenLB force-pushed the 192-define-evaluatorcomparer-interface branch from 52aaacd to 53a3ba0 Compare September 16, 2024 08:30
@JabobKrauskopf JabobKrauskopf linked an issue Sep 20, 2024 that may be closed by this pull request
Copy link
Contributor

@MarIniOnz MarIniOnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments and explanations needed! I really like the typing, makes everything much easier to understand!

Copy link
Member

@JabobKrauskopf JabobKrauskopf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noice! Much better. Some stuff as discussed in person

medmodels/medrecord/types.py Outdated Show resolved Hide resolved
medmodels/statistic_evaluations/comparer/data_comparer.pyi Outdated Show resolved Hide resolved
medmodels/statistic_evaluations/comparer/data_comparer.pyi Outdated Show resolved Hide resolved
Copy link
Contributor

@MarIniOnz MarIniOnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still need my previous comments to be resolved!

medmodels/medrecord/types.py Show resolved Hide resolved
medmodels/statistic_evaluations/comparer/data_comparer.pyi Outdated Show resolved Hide resolved
MarIniOnz
MarIniOnz previously approved these changes Oct 11, 2024
Copy link
Contributor

@MarIniOnz MarIniOnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DALLE2024-10-1111 24 48-Asealtheanimalappearinginsideacomputerscreengivingathumbsup Thesealshouldhaveafriendlysmilingexpressionandbepartiallyvisib-ezgif com-webp-to-jpg-converter

Copy link
Member

@JabobKrauskopf JabobKrauskopf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed in slack

Copy link
Contributor

@MarIniOnz MarIniOnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome change, I think it has a better interface this way!

A couple of comments!

@LauraBoenchenLB LauraBoenchenLB force-pushed the 192-define-evaluatorcomparer-interface branch from 6e12c88 to 51cea4e Compare October 17, 2024 15:19
Copy link
Member

@JabobKrauskopf JabobKrauskopf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval to get things rolling

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.

Define Evaluator/Comparer interface
3 participants