Progress calculation methodology integration

MaiaPelletier commented 2 years ago

Q	A
Feature branch/test site URL
Fixed issues	N/A
Related version	2.1.0-dev
Bugfix, feature or docs?	Feature

[ ] Keeps backward-compatibility
[ ] Includes tests
[ ] Updated docs
[ ] Updated config forms
[ ] Added CHANGELOG entry

Hi Brock - I know it's been a while since the last time we talked about adding this feature as a contribution, so my apologies. We've been really busy this year!

I used your recommendation for where to plug in my code for calculating indicator progress using our methodology that I outlined when we last met. A couple things to note about this PR:

Currently, it only handles the calculation of progress for indicators that have a total line aggregate. For any indicator that only reports disaggregated data, the calculation returns None. In the next version, I will be adding averaging of the calculations of progress for these indicators.
The progress labels that are output align with our methodology so they don't currently line up with the 4 categories that are available by default in Open SDG. We think it's still worthwhile to output the progress labels as part of the indicator meta property so that users can reference their progress status using the methodology.
We intend on providing more documentation for this feature in the next version as well.

Don't hesitate with any suggestions or questions once you've had a chance to review 🙂

codecov[bot] commented 2 years ago

Codecov Report

Patch coverage: 13.63% and project coverage change: -1.35 :warning:

Comparison is base (1f01711) 70.32% compared to head (1f82b44) 68.98%.

:exclamation: Current head 1f82b44 differs from pull request most recent head 6d3d128. Consider uploading reports for the commit 6d3d128 to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## 2.1.0-dev #318 +/- ## ============================================= - Coverage 70.32% 68.98% -1.35% ============================================= Files 74 75 +1 Lines 4536 4646 +110 ============================================= + Hits 3190 3205 +15 - Misses 1346 1441 +95 ``` | [Impacted Files](https://app.codecov.io/gh/open-sdg/sdg-build/pull/318?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-sdg) | Coverage Δ | | |---|---|---| | [sdg/ProgressMeasure.py](https://app.codecov.io/gh/open-sdg/sdg-build/pull/318?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-sdg#diff-c2RnL1Byb2dyZXNzTWVhc3VyZS5weQ==) | `11.32% <11.32%> (ø)` | | | [sdg/outputs/OutputOpenSdg.py](https://app.codecov.io/gh/open-sdg/sdg-build/pull/318?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-sdg#diff-c2RnL291dHB1dHMvT3V0cHV0T3BlblNkZy5weQ==) | `87.61% <75.00%> (-0.47%)` | :arrow_down: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

brockfanning commented 2 years ago

@MaiaPelletier Amazing, thank you!

This looks great to me. I just want to think out loud about the question of the progress types. I think clearly there need to be exactly 4 progress types, to cover the 4 conditionals at the end. Do you think though that the strings for these 4 progress types could be configurable? In Open SDG we have those as configurable in the site configuration, so it would be nice if they could be configurable here too. I'm imagining a configuration that might look something like:

progress_status_types:
  status_1: substantial_progress
  status_2: moderate_progress
  status_3: negligible_progress
  status_4: deterioration

That would also allow for any countries that might (for whatever reason) want to, for example, omit "negligible_progress":

progress_status_types:
  status_1: substantial_progress
  status_2: moderate_progress
  status_3: moderate_progress
  status_4: deterioration

Another question I have (maybe related) is do you think it would be helpful to have a configuration that is common across all indicators, and not indicator-specific? If so, my advice there would be to model it after something like the indicator_downloads option. If you look in this file, everywhere that you see indicator_downloads you could add something like progress_calculation or whatever. And then in you could pass it in with the call to OutputOpenSdg and then refer to it in that class by passing it to your calculation function. Just a thought - if you think that in practice every indicator would need completely different configuration, that is probably not needed. But if you think that a "common" configuration would be helpful then that is one way to do it.

MaiaPelletier commented 2 years ago

To your second point (and related to the first one), I do think that a set of common configurations across all indicators could be useful. The problem with the progress statuses is that the 2 methodologies (indicators with or without a numeric target) need to return a different set of progress statuses because of the different ways they measure progress (comparing against the actual target value vs. extrapolating). So if we were to have a common configuration across all indicators, we would need to allow users to configure 2 different sets of progress statuses, i.e.

status_types_target:
    - status_1: substantial_progress
    - status_2: moderate_progress
    - status_3: negligible_progress
    - status_4: deterioration

status_types_no_target:
    - status_1: substantial_progress
    - status_2: moderate_progress
    - status_3: moderate_deterioration
    - status_4: significant_deterioration

And even then, there would still be 6 potential outputs, which I know doesn't align with the 4 conditions that Open SDG currently supports by default. I noticed in the open sdg issue 'Progress' updates to goal pages · Issue #1747 · open-sdg/open-sdg (github.com) that these number of defaults might be changing in an upcoming release. This could potentially align better with the current set of progress statuses provided in our methodology?

LucyGwilliamAdmin commented 2 years ago

@MaiaPelletier @brockfanning some initial thoughts on aligning the progress statuses:

It would be great if we could align them, however, at the moment I feel we are reporting two different types of progress:

For the MVP we've been developing, progress relates to the final data point available compared to a target value
The methodology used direction of travel/trend (right?)

Therefore, I'm not sure how easy it would be to align them. I'm not sure that I see that as an issue though, more like we would be providing another option in terms of reporting progress. That being said, we would need to consider how the design may differ between the two types of progress

open-sdg / sdg-build

Progress calculation methodology integration #318

[ ] Added CHANGELOG entry

Codecov Report