-
Notifications
You must be signed in to change notification settings - Fork 7
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
New function to supersede Summarize
function to summarise results -- with more flexibilitythan previous utility function
#1457
Conversation
… that function (without disturbing the original summarize function, upon which many analayis script rely.)
…ummarize` to `summarise`
…se new function to avoid duplication
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.
Hi Tim. The PR looks great!
Find below my suggestions which I feel are all optional. Thanks
src/tlo/analysis/utils.py
Outdated
:Param: results: The pd.DataFame of results. | ||
:Param: central_measure: The name of the central measure to use - either 'mean' or 'median'. | ||
:Param: width_of_range: The width of the range to compute the statistics (e.g. 0.95 for the 95% interval). | ||
:Param: collapse_columns: Whether to simplify the columnar index if there is only one run (cannot be done otherwise) | ||
:Param: only_central: Whether to only report the central value (dropping the range). |
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.
:Param: results: The pd.DataFame of results. | |
:Param: central_measure: The name of the central measure to use - either 'mean' or 'median'. | |
:Param: width_of_range: The width of the range to compute the statistics (e.g. 0.95 for the 95% interval). | |
:Param: collapse_columns: Whether to simplify the columnar index if there is only one run (cannot be done otherwise) | |
:Param: only_central: Whether to only report the central value (dropping the range). | |
:param results: The dataframe of results to compute summary statistics of. | |
:param central_measure: The name of the central measure to use - either 'mean' or 'median'. | |
:param width_of_range: The width of the range to compute the statistics (e.g. 0.95 for the 95% interval). | |
:param collapse_columns: Whether to simplify the columnar index if there is only one run (cannot be done otherwise). | |
:param only_central: Whether to only report the central value (dropping the range). | |
:return: A dataframe with computed summary statistics. |
Small update to fix parameter directive syntax in docstring and adding return information.
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.
Needs lowercase p in param
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.
it is now a lowercase piece (I believe). This view seems to be out-dated.
Co-authored-by: Emmanuel Mnjowe <32415622+mnjowe@users.noreply.github.com>
Co-authored-by: Emmanuel Mnjowe <32415622+mnjowe@users.noreply.github.com>
Co-authored-by: Matt Graham <matthew.m.graham@gmail.com>
Co-authored-by: Matt Graham <matthew.m.graham@gmail.com>
Summarise
function to summarise results -- with more flexibilitythan previous utility functionSummarize
function to summarise results -- with more flexibilitythan previous utility function
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.
Thanks @tbhallett for the updates, this looks all good to me now!
Fixes #1420