2kde use hdi_probs by default

[aloctavodia]

Closes #2382

An "extra" contour is plotted by default, when passing hdi_probs or when passing an integer to levels. This little change "fix" the issue. Not entirely happy with the solution

---

📚 Documentation preview 📚: https://arviz--2383.org.readthedocs.build/en/2383/

[OriolAbril]

I agree with changing the behaviour with hdi_probs, I'd keep the same behaviour with int levels though to follow the "path of less surprise". I think users expect contour_kwargs and contourf_kwargs to be passed as is to matplotlib after adding some defaults, this would change that.

I remembered hdi_probs were converted into levels so it means we can change the hdi_probs->levels translation to fix the issue (if we restrict ourselves to that). Here https://github.com/arviz-devs/arviz/blob/main/arviz/plots/kdeplot.py#L282 both 0 and the maximum are added to the levels which with the new behaviour I think we only want for contourf but not for contour, so changing this to put a slice of the variable instead should fix the issue: https://github.com/arviz-devs/arviz/blob/main/arviz/plots/kdeplot.py#L292

[aloctavodia]

What you suggest will fix the issue when passing hdi_probs, actually that was my first thought too. But to my surprise if you specify countour_kwargs={"levels":x}, countourf_kwargs={"levels":x}, where x is an integer, or if you just do the default plot, you will still get the "extra" contour.

In other words, this is not something exclusive of hdi_probs.

[OriolAbril]

if you specify countour_kwargs={"levels":x}, countourf_kwargs={"levels":x}, where x is an integer, or if you just do the default plot, you will still get the "extra" contour.

The thing is this new "extra" contour is matplotlib new default behaviour. It might be a bit surprising at first because it has been added recently, but I think this is a temporal effect which will soon be reversed. People will have done/seen contourplots with matplotlib which show this outer contour delimiting where there is data but using the same parameters they won't get it if using ArviZ, in fact, depending on what we do it might even be impossible to get this behaviour at all if using ArviZ.

With hdi_probs the expected behaviour is clearer and unless someone asks for 0 as hdi_prob to be plotted that outer contour should not be there. But with integer value passed to levels the matplotlib docs say "If an int n, use MaxNLocator, which tries to automatically choose no more than n+1 "nice" contour levels", plus the fact that it is/was expected contour(levels=x) and contourf(levels=x) basically produce the same plot also makes things quite challenging.

I have gone quite a bit off-topic, but I think it mostly boils down to "do we actively dislike this outer contour and want to diverge from matplotlib behaviour?"

[aloctavodia]

I see your point, but my visceral dislike for this outer contour is greater, haha. What if we default to some values of hdi_probs other than None like [0.5, 0.8, 0.94]?

[OriolAbril]

What if we default to some values of hdi_probs other than None like [0.5, 0.8, 0.94]?

Sounds good

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.95%. Comparing base (8e14fc1) to head (5fba0bd).
Report is 10 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2383      +/-   ##
==========================================
- Coverage   87.00%   86.95%   -0.05%     
==========================================
  Files         124      124              
  Lines       12877    12882       +5     
==========================================
- Hits        11203    11202       -1     
- Misses       1674     1680       +6     

Flag	Coverage Δ
	86.95% <100.00%> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[aloctavodia]

@OriolAbril I think this is ready to merge.