-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: interface for comparer and builder #198
Conversation
b3b4afc
to
3e202d9
Compare
4f8139d
to
742f403
Compare
There was a problem hiding this 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More comments
medmodels/statistic_evaluations/statistical_analysis/inferential_statistics.pyi
Outdated
Show resolved
Hide resolved
medmodels/statistic_evaluations/statistical_analysis/inferential_statistics.pyi
Outdated
Show resolved
Hide resolved
medmodels/statistic_evaluations/statistical_analysis/inferential_statistics.pyi
Outdated
Show resolved
Hide resolved
52aaacd
to
53a3ba0
Compare
There was a problem hiding this 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!
medmodels/statistic_evaluations/statistical_analysis/descriptive_statistics.pyi
Outdated
Show resolved
Hide resolved
medmodels/statistic_evaluations/statistical_analysis/inferential_statistics.pyi
Outdated
Show resolved
Hide resolved
medmodels/statistic_evaluations/statistical_analysis/inferential_statistics.pyi
Outdated
Show resolved
Hide resolved
medmodels/statistic_evaluations/statistical_analysis/inferential_statistics.pyi
Outdated
Show resolved
Hide resolved
medmodels/statistic_evaluations/statistical_analysis/inferential_statistics.pyi
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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
385ac06
to
0f11c42
Compare
There was a problem hiding this 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!
0f11c42
to
5b7103a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
medmodels/statistic_evaluations/statistical_analysis/inferential_statistics.pyi
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
discussed in slack
There was a problem hiding this 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!
6e12c88
to
51cea4e
Compare
There was a problem hiding this 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
4b967d5
into
epic/157-create-an-evaluatorcomparer
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: