Add hacker-stats notebooks #98
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
There was a problem hiding this comment.
Cool beans, thanks @hugobowne!
I've dropped a few comments here that should help with organizing things nicely.
The only one I couldn't drop a comment for is the filenames for the notebooks; could you give them a shorter names, perhaps "01-simulated-probability.ipynb" and "02-parameter-estimation.ipynb"? Doing so makes managing the mkdocs.yml file navigation section just that teeny bit easier.
| @@ -0,0 +1,74 @@ | |||
| import pandas as pd | |||
There was a problem hiding this comment.
I think the entire data.py can be placed inside the project's src/bayes_tutorial/data.py; there's some data loaders already in there. Inside the notebooks, it should be a small change to get the data imported:
from bayes_tutorial.data import <the necessary functions>There was a problem hiding this comment.
HBA to move all data imports into src/bayes_tutorial/data.py (w/out duplication).
Edit import calls in NBs
| @@ -0,0 +1,5 @@ | |||
| axes.spines.left : True # display axis spines | |||
There was a problem hiding this comment.
I like the style sheet; let's get this one used across the entire series.
According to the Matplotlib docs, we can reference it from a single source of truth using the following pattern:
import matplotlib.pyplot as plt
plt.style.use('./images/presentation.mplstyle')I'd like to propose that we use the style sheet by doing the following:
- Place the
matplotlibrcfile in the project root directory. - Reference it at the top of every notebook (I'll take care of that for the other notebooks I've written), using the following pattern:
import matplotlib.pyplot as plt
from pyprojroot import here
plt.style.use(here() / "matplotlibrc")What do you think, @hugobowne?
There was a problem hiding this comment.
HBA to move matplotlibrc into config/matplotibrc
and load at top of both NBs.
| @@ -0,0 +1,19 @@ | |||
| import numpy as np | |||
There was a problem hiding this comment.
The utils.py module can probably be moved to the path src/bayes_tutorial/utils.py, which would allow us to use it across all of the notebooks.
| def despine(ax): | ||
| ax.spines['right'].set_visible(False) | ||
| ax.spines['top'].set_visible(False) |
There was a problem hiding this comment.
With the matplotlibrc file in place, do we need this function? If so, we keep it.
There was a problem hiding this comment.
I don't think I wrote any of this 👯♂️
| def despine_traceplot(traceplot): | ||
| for row in traceplot: | ||
| for ax in row: | ||
| despine(ax) |
There was a problem hiding this comment.
Ditto with this function; if we have matplotlibrc, is this function still needed? If so, we keep it.
@ericmjl as discussed in #97.
These NBs run programmatically but there's the small issue of data access: I think I need to mimic something you did and use some utility functions in the NBs to load the data?
We can discuss tomorrow but it may be worth adding some instructions to #97 so others can reproduce, if and when necessary.
Cool!