Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

View / edit / reply to this conversation on ReviewNB

OriolAbril commented on 2021-10-13T04:36:22Z
----------------------------------------------------------------

I think xarray can generate the scatterplot directly provided the posterior predictive samples are in the inferencedata, a bit like the scatter in https://pymc-examples.readthedocs.io/en/latest/case_studies/multilevel_modeling.html?highlight=multilevel#conventional-approaches.

drbenvincent commented on 2021-10-23T17:53:25Z
----------------------------------------------------------------

Thanks. This saves a good few lines of code. Have implemented this now.

View / edit / reply to this conversation on ReviewNB

OriolAbril commented on 2021-10-13T04:36:22Z
----------------------------------------------------------------

if using the pp from the inferencedata, this can be mean over named dims

drbenvincent commented on 2021-10-23T18:06:06Z
----------------------------------------------------------------

This is done now

View / edit / reply to this conversation on ReviewNB

OriolAbril commented on 2021-10-13T04:36:24Z
----------------------------------------------------------------

links to other notebooks should use sphinx cross-references. This one I think should be:

{doc}`diagnostics_and_criticism/Diagnosing_biased_Inference_with_Divergences`

ref: https://docs.readthedocs.io/en/stable/guides/cross-referencing-with-sphinx.html#the-doc-role

drbenvincent commented on 2021-10-23T16:33:08Z
----------------------------------------------------------------

This should now be done

Great notebook! Thanks! I added a couple comments plus the usual follow the style guide, specially add the post directive: https://github.com/pymc-devs/pymc/wiki/PyMC-Jupyter-Notebook-Style-Guide.

I also have a bit of a general comment. The style and writing of the notebook is very beginner friendly, trying to not assume much, which is probably best to appeal to non pymc/non bayes people and potentially drive clients to pymc labs, but I find the notebook a bit too long to be beginner friendly. I am not sure people will go over 6 full developed examples unless they are already very invested in pymc. None of the things are bad per se, I just wanted to mention it as you'll also now have to choose which category to use for the notebook and I wasn't completely sure how to review some points myself without knowing that

This should now be done

View entire conversation on ReviewNB

Thanks. This saves a good few lines of code. Have implemented this now.

View entire conversation on ReviewNB

This is done now

View entire conversation on ReviewNB

Thanks for the feedback @OriolAbril. As far as I can tell this should be all of your comments addressed now. Let me know if there's anything else :)

Anything else needed on this one before we can merge?

can you update it to follow the latest additions tot the style guide? https://pymc--5224.org.readthedocs.build/en/5224/contributing/jupyter_style.html (it is already available in latest too, but there is has some rendering issues)

I think the relevant sections are first cell and authorship ones. I would also recommend using more "conceptual" tags in addition to case study. Maybe "binned data", something about trying to make inference on populations? It might also be interesting for readers to have a pymc labs tag so notebooks about the work there are easy to find, and being an institutional partner I think it's perfectly fine.

Also sorry we move the finish line and sorry I missed your latest round of updates. I'll add discussing reviews here at pymc-examples to tomorrow's doc meeting, as it looks like me missing it meant a month of no reviews. Am I the only reviewer?

Notebook is up at https://docs.pymc.io/projects/examples/en/latest/case_studies/binning.html