Skip to content
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

ENH: move inflation_history to economic data #370

Merged
merged 19 commits into from
Feb 16, 2024
Merged

ENH: move inflation_history to economic data #370

merged 19 commits into from
Feb 16, 2024

Conversation

mmcky
Copy link
Contributor

@mmcky mmcky commented Feb 15, 2024

This PR moves the inflation_history lecture up to section on Economic Data

This PR also reviews inflation_history and makes the following adjustments

  • data is now retrieved from github instead of locally
  • data is now available for download
  • spell check and minor edits to language
  • allowed line graphs to be colourful (rather than gray)
  • numbered figures
  • changed the moving average mean to be centered=True so you use t-1,t, and t+1 in the calculation
  • updated the legend to name the line graph in the country inflation plots
  • incorporate feedback from [inflation_history] Review comments  #371
  • adjust global fig size and figure font size
  • remove embedded text from figures
  • suggested edits

Copy link

netlify bot commented Feb 15, 2024

Deploy Preview for taupe-gaufre-c4e660 ready!

Name Link
🔨 Latest commit c2f0f74
🔍 Latest deploy log https://app.netlify.com/sites/taupe-gaufre-c4e660/deploys/65cf1e06c91e5f000876a429
😎 Deploy Preview https://deploy-preview-370--taupe-gaufre-c4e660.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

github-actions bot commented Feb 15, 2024

@mmcky mmcky added the ready label Feb 15, 2024
@mmcky mmcky closed this Feb 15, 2024
@mmcky mmcky reopened this Feb 15, 2024
Copy link

github-actions bot commented Feb 15, 2024

@github-actions github-actions bot temporarily deployed to pull request February 15, 2024 23:58 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 16, 2024 00:15 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 16, 2024 00:25 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 16, 2024 01:04 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 16, 2024 01:17 Inactive

Each governent stoppped printing money to pay for goods and services once again made its currency convertible to the US dollar or the UK pound, thereby vitually to gold.
Each government stopped printing money to pay for goods and services once again which made its currency convertible to the US dollar or the UK pound.
Copy link
Contributor Author

@mmcky mmcky Feb 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jstac this is perhaps the most controversial edit re: gold comment. Should we keep thereby vitally to gold.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mmcky . I think your sentence is good but please change "which" to "and".

@github-actions github-actions bot temporarily deployed to pull request February 16, 2024 02:18 Inactive

```{code-cell} ipython3
import numpy as np
import pandas as pd
import matplotlib.pyplot as plt
plt.matplotlib.rcParams['figure.figsize'] = (12,8)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not set to a standard value in the config file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this matplotlib config needs to be initialised each time matplotlib is imported. We could add a global scale to the general config but that doesn't ensure the png files are high enough quality in all cases. Tricky one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW - we could look at using https://matplotlib.org/1.3.1/users/customizing.html and maintain a config file as it isn't essential for rendering in the notebook (just displaying on the website). This will add a bit of complexity around deployment though as it would need to be "installed"


As usual, we'll start by importing some Python modules.
We then import the Python modules we need.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think our convention is to have all imports and installs at the top. Shall we move this and the previous cell up and modify the text?

```

Now we write plotting functions so we can plot the price level, exchange rates,
and inflation rates, for each country of interest.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's good practice for us to name the funtions that will be hidden, and say roughly what they do, so readers who don't want to open the cell can still understand the code that follows.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will add to manual.quantecon.org

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


For each country, the second graph plots a three-month moving average of the inflation rate defined as $p_t - p_{t-1}$.
For each country, the second graph plots a centered three-month moving average of the inflation rate defined as $\frac{p_{t-1} + p_t + p_{t+1}}{3}$.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a moving average of prices or of the inflation rate?

Copy link
Contributor Author

@mmcky mmcky Feb 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inflation rate -- as the ma code is part of pr_plot or Plots for inflation rates

plt.show()
```

## Starting and Stopping Big Inflations

It is striking how **quickly** (log) price levels in Austria, Hungary, Poland,
and Germany leveled off after rising so quickly.
It is striking how **quickly** (log) price levels in Austria, Hungary, Poland, and Germany leveled off after rising so quickly.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bold is for defintions and italics is for emphasis: https://manual.quantecon.org/styleguide/writing.html#emphasis-and-definitions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will change to *

@jstac
Copy link
Contributor

jstac commented Feb 16, 2024

Thanks @mmcky , great changes! I've flagged some small issues above. Also, could you please bring the headings and axis labels in line with the manual?

Would you mind to get rid of all ax.spines[['right', 'top']].set_visible(False) type code, so that all figs are in a box? That way they'll all look the same.

Suggestion: "The graph shows..." -> "Figure xx shows..."

@mmcky
Copy link
Contributor Author

mmcky commented Feb 16, 2024

Thanks @jstac

Would you mind to get rid of all ax.spines[['right', 'top']].set_visible(False) type code, so that all figs are in a box? That way they'll all look the same.

Ah I went the other way and added it to all (in an effort to minimise lines) 😄 -- but I will remove the ax.spines code so there is a box around every figure.

@jstac
Copy link
Contributor

jstac commented Feb 16, 2024

Ah I went the other way and added it to all (in an effort to minimise lines) 😄 -- but I will remove the ax.spines code so there is a box around every figure.

Totally reasonable -- I like minimal -- but some of the figs have axis numbers on the right, so they end up being three sided, which looks a bit odd.

I think it's nice if every figure has a uniform style, and we can achieve this with boxes...

@mmcky
Copy link
Contributor Author

mmcky commented Feb 16, 2024

Suggestion: "The graph shows..." -> "Figure xx shows..."

Great. I updated this type of text to numref in a number of places when referencing a specific graph. There are some instances of graph that are more generally related to a group that I have left.

@github-actions github-actions bot temporarily deployed to pull request February 16, 2024 03:41 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 16, 2024 04:02 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 16, 2024 04:17 Inactive
@mmcky
Copy link
Contributor Author

mmcky commented Feb 16, 2024

thanks @jstac I think all round 2 feedback is now incorporated.

@jstac
Copy link
Contributor

jstac commented Feb 16, 2024

@mmcky please feel free to merge when you've made those last changes.

If it doesn't mess up the figures, perhaps we should also get rid of plt.rcParams.update({'font.size': 19}).

I think the lectures are smoother for readers if they don't hit messy commands at the very start of the lecture.

With figure sizes, let's strongly encourage people to stick with defaults unless they have a good reason not to --- and consider changing the defaults if we often need to deviate...

@mmcky
Copy link
Contributor Author

mmcky commented Feb 16, 2024

thanks @jstac e22b9de fixes the within figure text to be in style with the QuantEcon manual.

@github-actions github-actions bot temporarily deployed to pull request February 16, 2024 08:33 Inactive
@mmcky
Copy link
Contributor Author

mmcky commented Feb 16, 2024

@jstac I have updated figures to

  • simplify matplotlib config that is requried
  • migrate figure text to use matplotlib labels for x and y

@github-actions github-actions bot temporarily deployed to pull request February 16, 2024 08:42 Inactive
@mmcky mmcky merged commit 147d10f into main Feb 16, 2024
6 checks passed
@mmcky mmcky deleted the update-toc branch February 16, 2024 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants