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

Rethinking_2 Chp_04: update plot functions, flag trace_to_dataframe() deprecation #139

Merged
merged 2 commits into from
Jan 7, 2021

Conversation

joannadiong
Copy link
Contributor

Hi there. Here are some updates:

  • added random seed
  • ax.imshow() keyword changed from bottom to lower
  • az.plot_hpd() updated to az.plot_hdi() in various places
  • Code 4.61 and elsewhere: manually pass plot axes so plots of raw data, CI and PI overlay, as per this recent Issue. Interestingly, I could only overlay the az plots with matplotlib plots provided the az functions are run first, followed by plt functions. Does it seem a little inflexible to make e.g. scatterplots always come after compatible/credible or prediction intervals? I have likely missed something so would appreciate feedback.
  • Code 4.78: plot ax plot first, then set fig dimensions
  • Code 4.32 and elsewhere: I read the InferenceData docs (with thanks to @AlexAndorra for links) but have retained the trace_to_dataframe() functions for now; fixing it feels a little beyond me and I don't want to break the code. I think the potential solution is below, but can't work out how to get the covariance estimates needed for this section. Happy for a developer to fix, or I could attempt a fix if someone could give pointers. Thanks.
# Code 4.32
with pm.Model() as model:
    pm_data = az.from_pymc3(        
        trace=trace_4_1
        )

Notes: For PyCharm users, in the .py export of the Notebook, PyCharm indicates that dmatrix can't be found in __init__.py but the code still runs. From old posts, this seems to be a bug.

Thanks also for the helpful PR template. pre-commit checks run and passed. I should probably break this up into small commits next time. I have tried to update (rebase) my fork of the repo correctly given there's many contributors. Please tell me if anything is awry, but hopefully everything works. Thanks for your patience as I'm on this interesting learning curve!

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@review-notebook-app
Copy link

View / edit / reply to this conversation on ReviewNB

canyon289 commented on 2021-01-07T01:28:39Z
----------------------------------------------------------------

For some reason the tables from az.summary are missing. Not sure why


@canyon289
Copy link
Member

Merging before any conflicts arise. Thanks so much for this contribution!

@canyon289 canyon289 merged commit 72999e4 into pymc-devs:master Jan 7, 2021
@joannadiong
Copy link
Contributor Author

Hi @canyon289, thanks for the speedy reply!

az.summary() tables
The tables do appear after the command in my Notebook (e.g. see figure) -- do they not appear at your end?
Screenshot from 2021-01-07 14-04-36

deprecated trace_to_dataframe()
The Notebook code currently retains this function but it will be deprecated in future releases so the Notebook will break. In brief discussion elsewhere with @aloctavodia and @AlexAndorra, one option for longer-term updates of the Notebooks is for learners to open PRs for each Chapter, but perhaps make a Work-In-Progress PR for larger fixes by the Developers or us (over time). Given that trace_to_dataframe() is still in the Notebook, what do you think about re-opening this PR or converting it to a WIP PR until the function is updated?

Many thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants