PSLmodels / OG-Core

An overlapping generations model framework for evaluating fiscal policies.
https://pslmodels.github.io/OG-Core/
Creative Commons Zero v1.0 Universal
66 stars 114 forks source link

Fix error converting to pct diffs #930

Closed jdebacker closed 5 months ago

jdebacker commented 5 months ago

In PR #914, I introduced an error into the output_plots.plot_aggregates function. In this case, I multiplied the pct diffs by 100 to get them in percentage points, forgetting that the plot formatting options used:

if plot_type == "pct_diff":
        ax1.set_yticklabels(["{:,.2%}".format(x) for x in vals])

already take care of this. Thus, the effect of my change was to make the pct diffs look 100 times as large.

This PR fixes that error.

codecov-commenter commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 72.76%. Comparing base (9bf8810) to head (62eb7d7). Report is 3 commits behind head on master.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/930/graphs/tree.svg?width=650&height=150&src=pr&token=98mQCVhspd&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels)](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/930?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels) ```diff @@ Coverage Diff @@ ## master #930 +/- ## ======================================= Coverage 72.75% 72.76% ======================================= Files 19 19 Lines 4643 4644 +1 ======================================= + Hits 3378 3379 +1 Misses 1265 1265 ``` | [Flag](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/930/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/930/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels) | `72.76% <100.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/930?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels) | Coverage Δ | | |---|---|---| | [ogcore/output\_plots.py](https://app.codecov.io/gh/PSLmodels/OG-Core/pull/930?src=pr&el=tree&filepath=ogcore%2Foutput_plots.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels#diff-b2djb3JlL291dHB1dF9wbG90cy5weQ==) | `95.34% <100.00%> (+0.01%)` | :arrow_up: |
rickecon commented 5 months ago

@jdebacker. This PR looks good. I am ready to merge this if you are done. And I like your addition to the codecov GH Action.

jdebacker commented 5 months ago

I've also gone ahead and made some changes to the output_plots.plot_all() function in this PR. In particular, I removed the G series from the percentage change in fiscal variables because the closure rule makes these look wacky as G gets close to zero.

So that users can see the G series, I added a plot to show the G/Y ration in the baseline and reform.

jdebacker commented 5 months ago

@rickecon Thanks for taking a look at this. I have now completed the changes I want to make on this PR.

rickecon commented 5 months ago

Merging