update Binomial regression notebook to v4

[drbenvincent]

#214

These changes should hopefully bring this notebook up to v4 spec. Just let me know if there's anything amiss.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

OriolAbril commented on 2022-01-30T23:54:30Z
----------------------------------------------------------------

notebook_id should be replaced by the keyword we want to use to reference this notebook, for example the filename or some other notebook specific keywords.

drbenvincent commented on 2022-02-01T16:42:16Z
----------------------------------------------------------------

Fixed in an imminent commit

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

OriolAbril commented on 2022-01-30T23:54:30Z
----------------------------------------------------------------

Line #13.    post_mean = idata.posterior.stack(sample=("chain", "draw")).p.mean("sample")

xarray supports averaging along multiple dimensions at the same time. As we are not really using the sample dimension, we can skip the stacking:

post_mean = idata.posterior.p.mean(("chain", "draw"))

drbenvincent commented on 2022-02-01T16:42:49Z
----------------------------------------------------------------

Thanks, this is handy to know! Still learning about the wonders of Xarray. Have replaced this in an imminent commit

[OriolAbril]

commented two nits, the main issue are the git conflicts with the references file

[OriolAbril]

you can also add a url field to include https://bookdown.org/roback/bookdown-BeyondMLR/ in the bibliography citation too.

[drbenvincent]

Done :)

[drbenvincent]

Fixed in an imminent commit

---

View entire conversation on ReviewNB

[drbenvincent]

Thanks, this is handy to know! Still learning about the wonders of Xarray. Have replaced this in an imminent commit

---

View entire conversation on ReviewNB

[drbenvincent]

Made those changes 👍

I'm not sure why I'm getting conflicts with the references.bib. Do I need to do a rebase before I create a new feature branch?

[OriolAbril]

The bibliography is not working, it looks like the whole directive is in a single line, it must be in 3 lines minimum like in the example at https://docs.pymc.io/en/latest/contributing/jupyter_style.html#references

[ltoniazzi]

Two tiny comments: (i) the Jupyter style guide suggests to not use unicode in code blocks; (ii) Successes is sometimes mis-spelled

[drbenvincent]

Thanks @ltoniazzi. I'm not a fan of the no unicode thing, but I've done that.

I've also added # hide-input comment at the start of two long cells of plotting code. This ability to have some code cells collapsed is really neat and show allow much more focus on the concepts.

[drbenvincent]

I'm running the pre-commit checks, but the new bibtex-tidy is showing up as skipped. So not sure what to do about that

[OriolAbril]

Try running pre-commit run --all, then commititng the changes. I think there is also a github command/shortcut but I never remember it.

[drbenvincent]

All checks have passed 🥳

[michaelosthege]

Looks good to me!

Also TIL that arviz.plot_hdi is an alternative/addition to pm.gp.util.plot_gp_dist.
On that side note: @OriolAbril what do you think about migrating pm.gp.util.plot_gp_dist to arviz and extending it with a kwarg to draw HDI limits kind of like this?

Shall I open an Issue/Discussion over at ArviZ ?

[OriolAbril]

I hardly ever use gps and don't know what pm.gp.util.plot_gp_dist is, but an issue/discussion sounds good. do you know why hasn't it been moved yet? does it rely heavily on the model or extra metadata not present in inferencedata?

[michaelosthege]

I hardly ever use gps and don't know what pm.gp.util.plot_gp_dist is, but an issue/discussion sounds good. do you know why hasn't it been moved yet? does it rely heavily on the model or extra metadata not present in inferencedata?

not at all. It's pretty generic and you can use if for any kinds of bands.
We probably forgot to move it because it's hidden in the GP submodule.

I'll open a ticket.