Bring back ArviZ wrappers with DeprecationWarnings

[michaelosthege]

@twiecki and I discussed about the bad user experience we accidentally caused by removing ArviZ wrappers.
Many users did not yet upgrade, so we have a chance to fix the UX and ease the transition.

Here's how:

• On the v3 branch we reintroduce the wrappers with the same API they had before. But we add warnings.warn() to nudge users towards one of the options beflow.
• On the master and v4 branches we replace the above warnings.warn() with a raise DeprecationWarning. This is better than removing the function entirely, because we can still communicate instructions.

Options for future wrappers:

1. pm.az: by introducing import arviz as az into the PyMC3 __init__.py
2. pm.plotting: by introducing import arviz as plotting into the PyMC3 __init__.py

[michaelosthege]

Personally I prefer 1.) because it keeps the ArviZ branding.
We can still have a pm.plotting for stuff that doesn't exist in ArviZ, but I don't think we should mix the two feature sets.

@AlexAndorra @OriolAbril @rpgoldman what do you think?

[OriolAbril]

what exactly was supported by wrappers? Only the pm.traceplot like features for backward compatibility of were all arviz functions accessible? I myself never used them and I think I am missing some context.

The main pain point for me related to wrappers is that they used to explicitly import arviz functions, so whenever we rename or remove one of the functions (i.e. hpd to hdi or removing geweke) two things happen: 1) we need to update the pymc codebase to take that into account, 2) old pymc versions that explicitly import the function get broken with latest arviz even though the from_pymc3 converter might still work. If possible, it would be great to have them in a try except so that running the code fails, but you can import and run pymc3 if you don't use that particular function

Note on geweke: we may have deleted geweke too fast and without warning, but the truth is that it had been basically broken for a while and nobody complained. The complains where only about latest arviz breaking pymc 3.8 or 3.9 installation I think it was, which turned out to be the default versions installed on python 3.6 😕

[michaelosthege]

pm.stats and pm.plots were edited in #4397.

The change was released in 3.11.0.

We can restore pm.plots and pm.stats with plenty of warnings and the try/except thing, just for compatibility.
But because of the renaming problems @OriolAbril mentioned, we should just delegate everything through pm.az.

[rpgoldman]

I like the pm.az idea, as well.

Something that might make the linking happier would be to add to ArviZ CI some kind of simple test that will identify when changes break the existing stubs. I suppose that could also be used to update the requirements in pymc3 so that upgrades to breaking versions get blocked, or at least a warning?

@michaelosthege Isn't it FutureWarning we should use instead of DeprecationWarning for the stubs, since they are user-facing?

OT: Also, should we be using PendingDeprecationWarning in some cases? I don't know how we would decide.

[AlexAndorra]

I like pm.az better too -- as ArviZ is about statistical diagnostics as well, pm.plotting feels too restrictive.
For how long are we planning to have these wrappers again though? The goal was to get rid of them eventually and completely delegate to ArviZ (which is easier for both ArviZ's and PyMC's development), so are we reintroducing those wrappers with prominent deprecation warnings for a few versions, and then cutting them out again?

[twiecki]

I think this only makes sense if we keep the old API for v3, without warnings. For me the reason is that blog posts and old code keeps running with v3. If we already have a clean cut with v4 where we break things we should offload things to that.

[AllenDowney]

Having been tripped up by this, I agree that API changes like this should start with a DeprecationWarning.

[ricardoV94]

Agree with just reverting for the V3. No point in introducing yet another (user facing) change at this point if the point is to just leave this branch in a stable state for the future months. We can do a bit of work to make sure internally the code is not so dependent on arviz name changes (I guess this is what the try/except thing is about?).

[michaelosthege]

Thanks for the discussion!
@rpgoldman I will look up guidance on when to use DeprecationWarning vs. FutureWarning vs. FutureDeprecationWarning.

I think we can safely introduce the new wrapper (pm.az) on v3 as it will not "break", because it's just an alias. Then we can have warnings in the old pm.stats and pm.plotting wrappers that nudge people towards the v4-compatible pm.az.

@ricardoV94 yes, I think the old pm.stats and pm.plotting are inherently unstable and we can at least try to stabilize them. Still, nudging towards pm.az brings better stability and reduces the friction between v3→v4,

[ricardoV94]

What's the point of pm.az (for the V4) though? Save typing in importing arviz?

[michaelosthege]

What's the point of pm.az (for the V4) though? Save typing in importing arviz?

Some, specifically @twiecki, argue that it's a feature that many (beginner) users ask for & that a wrapper is less confusing than having to use a separate library.

[AlexAndorra]

So I'm a bit lost now: is the pm.az wrapper only for v3? And we completely delegate to ArviZ in v4?

[AllenDowney]

I agree with @twiecki on this point. I spend a lot of time explaining things like this to beginners whose cognitive capacity is completely tapped. Every detail like this that I don't have to explain creates capacity to learn things that matter.

[michaelosthege]

So I'm a bit lost now: is the pm.az wrapper only for v3? And we completely delegate to ArviZ in v4?

I'd put the pm.az in both. And yes, we completely delegate to ArviZ (in both), because we don't even wrap anything; just alias pm.az == arviz.

[ricardoV94]

Honestly pm.az seems even more enigmatic than importing arviz as az, since az has no meaning on it's own (unlike say pm.math, which makes it obvious what is "inside"). At least with the separate import I know where to look for documentation.

I have nothing against it (specially if it eases maintenance due to naming conflicts), I just doubt it will provide any improved user experience.

[michaelosthege]

Honestly pm.az seems even more enigmatic than importing arviz as az, since az has no meaning on it's own (unlike say pm.math, that makes it obvious what is "inside"). At least with the separate import I know where to look for documentation.

I have nothing against it, I just doubt it will provide any improved user experience.

If you ask me the whole aliasing numpy as np, pandas as pd, pymc3 as pm, arviz as az are bad tradeoffs between characters and code clarity. I was super confused by it when I started with Python and so are my colleagues & students when they take their first steps. In our codebases we don't even do the aliasing and I think it's a lot easier to read.

I will use pm.az or pm.arviz; whatever you prefer.

Also I'll open a PR for this issue today or tomorrow.

[twiecki]

Yes, I'm also strongly opposed to pm.az for the same reason @ricardoV94 provides.

And I also couldn't have worded the main motivation better than @ricardoV94 here:

Agree with just reverting for the V3. No point in introducing yet another (user facing) change at this point if the point is to just leave this branch in a stable state for the future months. We can do a bit of work to make sure internally the code is not so dependent on arviz name changes (I guess this is what the try/except thing is about?).

We want to get back to the UI everyone is used to, the UI that is present in hundreds of blog posts, lectures, and books, not introduce yet a new one we have to explain and teach that might again change with v4. We want consistency and predictability to keep the trust of our users, the idea here is to get some of that back.

[michaelosthege]

@twiecki but what does that mean for v4 should it get any Arviz alias/wrapper at all? And if it's an alias anyway why now backport it?

[twiecki]

@michaelosthege For v4 I would say to have pymc3.plot be an alias to arviz.plot (like pm.math) and pymc3.stats be an alias to arviz.stats.

[twiecki]

That way, there's less cognitive overhead for newcomers, we don't have to maintain wrappers, and there's a smooth transition from v3 where we just have to say that pm.traceplot is now pm.plot.plot_trace() (or pm.plot_trace()).

[michaelosthege]

After this issue was resolved for v3 in #4536 I changed the milestone to v4.