Bring back stats and plotting aliases #4536
Conversation
pymc3/plots/__init__.py
Outdated
| densityplot = map_args(az.plot_density) | ||
| pairplot = map_args(az.plot_pair) | ||
|
|
||
| # Use compact traceplot by default |
There was a problem hiding this comment.
ArviZ switched their default in v0.11.0 and pinned that as a minimum.
pymc3/plots/__init__.py
Outdated
| """ | ||
| import functools | ||
| import sys | ||
| import warnings | ||
|
|
||
| import arviz as az | ||
|
|
||
|
|
||
| def map_args(func): | ||
| swaps = [("varnames", "var_names")] |
There was a problem hiding this comment.
These should have been around for long enough - I replaced the soft with a hard warning.
pymc3/stats/__init__.py
Outdated
| r2_score = map_args(az.r2_score) | ||
| rhat = map_args(az.rhat) | ||
| summary = map_args(az.summary) | ||
| waic = map_args(az.waic) |
There was a problem hiding this comment.
All the names are identical - therefore they can be imported without wrapping.
|
As you can see there are two commits. The first is the revert of the one that removed the wrappers a while back. In the second commit I straightened and simplified the API and implementation. (We'll probably want to squash-merge.) |
michaelosthege
force-pushed
the
fix-v3-4528
branch
from
f25affb
to
b98a895
Compare
docs/source/api/stats.rst
Outdated
| - :func:`pymc3.bfmi <arviz:arviz.bfmi>` | ||
| - :func:`pymc3.compare <arviz:arviz.compare>` | ||
| - :func:`pymc3.ess <arviz:arviz.ess>` | ||
| - :data:`pymc3.geweke <arviz:arviz.geweke>` |
There was a problem hiding this comment.
we deleted geweke in ArviZ, it had basically been broken for a while (worked only on 1d arrays) and nobody had complained. This is why there are questions on discourse about it being missing, but so far nobody has complained about it being missing, only about the import error it causes with old pymc3 versions.
docs/source/api/stats.rst
Outdated
| - :func:`pymc3.compare <arviz:arviz.compare>` | ||
| - :func:`pymc3.ess <arviz:arviz.ess>` | ||
| - :data:`pymc3.geweke <arviz:arviz.geweke>` | ||
| - :func:`pymc3.hpd <arviz:arviz.hpd>` |
There was a problem hiding this comment.
I understand wanting to keep the original pm.hpd name, but az.hpd no longer exists either, it is now az.hdi. This one is used and had a deprecation warning for several arviz releases indicating users to switch from hpd to hdi.
Note that currently the code does not do any hpd-hdi aliasing on pymc3 side though.
|
I like it, but I'm not a fan of the deprecation warning. For me the ideal path forward is:
If we agree on this, we don't need deprecation warnings here, except for maybe that the names will change from |
|
Terminology:
As soon as the original ArviZ function name is used, there is no |
|
Ah, perfect! So calling |
|
|
Awesome, same page. |
Co-authored-by: Oriol Abril-Pla <oriol.abril.pla@gmail.com>
Co-authored-by: Oriol Abril-Pla <oriol.abril.pla@gmail.com>
|
For some reason I couldn't use "Add suggestion to batch". |
Also see pymc-devs#4536 where the wrappers were brought back for v3. Closes pymc-devs#4528
Also see pymc-devs#4536 where the wrappers were brought back for v3. Closes pymc-devs#4528
Also see pymc-devs#4536 where the wrappers were brought back for v3. Closes pymc-devs#4528
As discussed in #4528 this PR brings back the old
pm.plottingandpm.statssubmodules.However they now have deprecation warnings that instruct users to switch to the original function names, either via
pm.*orarvizdirectly.Depending on what your PR does, here are a few things you might want to address in the description:
what are the (breaking) changes that this PR makes?It un-breaks!important background, or details about the implementationAll objects except__*ones are stolen from thearviz.statsorarviz.plotssubmodules. This is to makepm.plotsandpm.statsas identical as possible, so users can seamlessly switch to the original ArviZ submodules.are the changes—especially new features—covered by tests and docstrings?They are covered in ArviZ.consider adding/updating relevant example notebooksNo.