Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Update some examples to use new arviz#822

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
aloctavodia wants to merge17 commits intomain
base:main
Choose a base branch
Loading
fromarviz
Open

Conversation

@aloctavodia
Copy link
Member

No description provided.

@review-notebook-app
Copy link

Check out this pull request on ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered byReviewNB

Copy link
Member

@OriolAbrilOriolAbril left a comment

Choose a reason for hiding this comment

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

other than the extra files with.md instead of.myst.md extension it looks good

all += perct_per_class
print(perct_per_class)
all
az.plot_ppc_pava(idata_t, data_type="categorical");
Copy link
Member

Choose a reason for hiding this comment

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

My main comment is probably not actionable but might be worth to keep in mind for arviz development. I am not sure I can see this as it is a comparison between this figure and one a few sections higher. Doesplot_ppc_pava support dict input to show both models, if not, do you think that is useful to add?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I think the only ones supporting more than one model are plot_forest and plot_dist. Not sure how useful, it could be ok forplot_ppc_pava and maybeplot_ppc_pit. Maybe something more general is to have something similar tocombine_plots but that works for different data. So we can create a single figure with one model per row or column, so comparisons are easier to see and present.

Copy link
Member

Choose a reason for hiding this comment

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

this one also seems to be an extra file. There might be something broken in the jupytext settings :/

)
ax[1].set(title="Posterior Predictive Check (test)", xlim=(0, 1_000));
```{code-cell} ipython3
az.plot_ppc_dist(posterior_predictive_oos_regression_test, kind="ecdf");
Copy link
Member

Choose a reason for hiding this comment

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

might be more distracting than helpful now that the plot is a single line, but we could keep the 0-1000 xlims with

Suggested change
az.plot_ppc_dist(posterior_predictive_oos_regression_test, kind="ecdf");
pc =az.plot_ppc_dist(posterior_predictive_oos_regression_test, kind="ecdf")
pc.get_viz("plot", "y").set_xlim(0, 1_000);

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

could be, but also important to show the syntax.

@fonnesbeck
Copy link
Member

arviz.preview? Do we want to wait on this until its no longer in preview? We will just have to go back and change this again, right?

@aloctavodia
Copy link
MemberAuthor

aloctavodia commentedDec 3, 2025•
edited
Loading

I think we should not wait. Once we have ArviZ 1.0, we will need to update one import. I will do the update.

Edit:
@fonnesbeck, not sure my previous answer was that useful. My main motivation with this PR is to check that "new arviz" has all the functions that we want and they work as expected, and that others can provide feedback. I think is Ok to merge this eventually, but fine with me if we want to wait for ArviZ 1.0 (or RC).

@OriolAbril
Copy link
Member

OriolAbril commentedDec 3, 2025•
edited
Loading

I think it is probably better to start the migration before the actual 1.0 release. After that we'll only need to replacearviz.preview byarviz. I think it will also help signal users they should start preparing for the migration which will make the update smoother.

Moreover, if we wait, we won't be able to update any page until pymc has fully migrated internally. That means there are many new features like dedicated ppc plots, prior sensitivity checks, loo with moment matching or subsampling, advanced facetting and aesthetic mappings that are ready to use but can't be used because we don't want to usearviz.preview.

@fonnesbeck
Copy link
Member

That sounds good to me.

@aloctavodia just a few failures that I would have expected the pre-commit hooks to have caught.

@aloctavodia
Copy link
MemberAuthor

The failures I omitted are related to links, not sure about the correct syntax.

@OriolAbril
Copy link
Member

I think the change I did will fix pre-commit (once I guess the right escaping rules). We were disallowing any links toarviz.github.io because old arviz docs where atarviz.github.io/arviz and we wanted to use sphinx cross-references instead,https://arviz-devs.github.io/EABM/ is also under the same root URL but it is a quarto website so we can't use the same cross-references trivially

@OriolAbril
Copy link
Member

the new error related to pyupgrade seems to be happening onmain too. No idea about the potential cause without properly looking into it. The logs from the failed run inmain might be more helpful that the ones in the PR though:https://github.com/pymc-devs/pymc-examples/actions/runs/20341400571/job/58441502049. It mentions "syntaxwarnings that will become errors"

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@OriolAbrilOriolAbrilOriolAbril left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aloctavodia@fonnesbeck@OriolAbril
[8]ページ先頭
©2009-2025 Movatter.jp