Allow newer arviz in pyodide #248
Conversation
There was a problem hiding this comment.
Compared the SIR model default Python analysis scripts on the proposed branch and the existing production.
Proposed changes run without error and produce equivalent graphs (within expected variation from the non-deterministic sampling process).
I did find one minor issue: I suspect there may have been a change in how Arviz handles default margins/axis scales and legends. I ran the same analysis script on prod (older Arviz) and the PR branch (newer Arviz). The analysis script plots the draws' predicted cases against day-since-first-infection. The plotted data was roughly equivalent: both runs peaked at day 6, with 332 cases for the PR branch and 327 for prod. Despite the similar data range, the plot from Production extends the Y-axis up to about 350, while the plot from the PR branch only goes up to about 330 (i.e. just enough to display the data). As a result, the PR branch's plot overlaps with the legend, while the branch on Production does not, so the new version looks worse.
Ultimately, this probably shouldn't be a blocker, but I'm mentioning it here to note that there may be subtle changes in how plotting code behaves between the two versions. But we can't stay on the old version forever, so I think we should accept this PR and let users figure out whatever tweaks they need to do.
|
That's certainly more thorough than what I did, which was just importing it to make sure the install still worked... It's a good spot though. Another item on the list of things we cannot (or at least, will not) promise, I think |
We were installing
arviz<0.18, because that version added a dependency ondm-tree, which wasn't WASM compatible. The latest arviz (0.20) has made that dependency optional, so the pinning is no longer necessary