rcpch / rcpch-audit-engine

Epilepsy12 Audit Platform
https://e12.rcpch.ac.uk/
GNU Affero General Public License v3.0
5 stars 5 forks source link

Adds Country column to Country - Sheet 2 #925

Closed dc2007git closed 3 months ago

dc2007git commented 3 months ago

Overview

Adds the column 'Country' to sheet number 2 - 'Country'

Code changes

Adds a line to the create_kpi_report_row function to render either 'England' or 'Wales' as the country and add it to the ret dictionary object

Documentation changes (done or required as a result of this PR)

N/A

Related Issues

Closes #924

mbarton commented 3 months ago

This works but I'd like to avoid putting sheet specific modifications low down in the call stack where we can. It makes it harder to understand what the low level function is doing without holding all the calling code in your head at the same time.

Could we add the extra column in kpi.py only for sheet 1? eg

kpi.py:94

england_row = create_kpi_report_row(
    "england", measure_title, kpi, england_kpi_aggregation
)

england_row["Country"] = "England"

wales_row = create_kpi_report_row(
    "wales", measure_title, kpi, wales_kpi_aggregation
)

wales_row["Country"] = "Wales"
dc2007git commented 3 months ago

My mistake thanks for the suggestion @mbarton. Will push the changes to this PR

mbarton commented 3 months ago

Thanks @dc2007git! It's not a mistake on your part, just a suggestion on mine :)

dc2007git commented 3 months ago

@mbarton the most recent changes I've pushed to this PR can be discarded, as well as with this entire PR, because this is solved in a more holistic way in PR #926 , which means we can remove the unnecessary columns, reorganise the columns, and add the uk column all in one fell swoop!