[WIP]Add de-aliaser function for plots

[percygautam]

Description

Fixes #1041 . Added function to dealiase the plot kwargs passed to plots by user. Working example can be found in cookbook.

Checklist

• Follows official PR format
• New features are properly documented (with an example if appropriate)?
• Code style correct (follows pylint and black guidelines)
• Changes are listed in changelog

[OriolAbril]

I would take advantage of the fact that alias_mapping argument in cbook.normalize_kwargs can be a subclass of Artist objects.

Therefore, cbook.normalize_kwargs(plot_kwargs, mpl.lines.Line2D) should do the trick and we can avoid maintaining the alias dicts. Regarding one or several functions, maybe we can wrap normalize_kwargs for several common objects Line2D (for plot), PathCollection (for scatter) and so on.

[percygautam]

Okay, I'd do the changes. We can use a single function for dealiasing.

[percygautam]

I would take advantage of the fact that alias_mapping argument in cbook.normalize_kwargs can be a subclass of Artist objects.

@OriolAbril This functionality is only present in unreleased latest version 3.2.0 and not in version used by arviz 3.1.2.
https://github.com/matplotlib/matplotlib/blob/3b2a89825e5e5a98b8b9478c8f103cc22b9b706f/lib/matplotlib/cbook/__init__.py#L1768-L1772

As a workaround I'd do something like this:

def dealiser(args): 
    return cbook.normalize_kwargs(args, getattr(mpl.collections.PathCollection,"_alias_map", {}))

We can use this workaround until arviz shifts to new version of matplotlib.

[OriolAbril]

The workaround sounds good, in this case it is probably better to define a handful of dealiasers (or a dealiaser with some kind argument) to allow dealiasing for both plot and scatter and also hexbin of hist if they were to return a different artist than plot and scatter

[percygautam]

I have applied dealiser to all the cases I can find. Please check if I missed something.
Also, what should we do in the case of compareplot?(we are defining arviz specific kwargs)

[OriolAbril]

Two comments only. I would name the function as something more explicit like matplotlib_kwarg_dealiaser or normalize_matplotlib_kwargs. Also, we have to make sure that after dealiasing, only the default prop name is used (e.g. no lw nor ms...)

[percygautam]

I have checked the files and confirmed that only default prop names are used.
Also, for case we set default by using alias, then kwargs dict will contain both alias prop(set by arviz) and dealiased prop (set by user), for such case full names are given precedence.( ref: https://github.com/matplotlib/matplotlib/blob/3b2a89825e5e5a98b8b9478c8f103cc22b9b706f/lib/matplotlib/cbook/__init__.py#L1722). So, only user-defined property will be used in plots.

Anyway, currently it won't cause any error and we can correct that alias aren't used in future .

[OriolAbril]

How do you feel about adding a couple of tests for matplotlib_kwarg_dealiaser?

[percygautam]

Should I add more test cases or are these enough?

[OriolAbril]

I think this test is enough. We are basically relying on matplotlib.

[percygautam]

The dealiasing function is not working for python 3.5. Any workaround?

[OriolAbril]

The dealiasing function is not working for python 3.5. Any workaround?

matplotlib 3.1 droped support for python 3.5, so CI uses 3.0.3 which probably has different alias. Checking the keys and for which kinds it fails should allow to remove the combination from tests

[OriolAbril]

@percygautam I just had an idea. Currently most plots do something similar to:

if kwargs is None:
    kwargs = {}
else:
    kwargs = matplotlib_kwarg_dealiaser(kwargs)

We could simplify this by making matplotlb_kwarg_dealiaser also accept None in which case it would return {} straight away and not call matplotlib's kwarg normalizer. All test suite was reorganized, so a rebase/merge from master is needed.

Note: Also, to avoid merge issues with #1079 that does a considerable rewrite of plot_pair, it'd be better to not modify pairplot.py.

[percygautam]

I've done the changes and left pairplot as before.

[OriolAbril]

It looks like you have removed some blank lines in plot_pair docstring and now it the lint complains.

I have one question about matplotlib/bokeh interaction. What happens if there are keys in the kwargs dict that do not correspond to any alias nor accepted argument? is there any warning/error? Bokeh will have different names, so the dealiaser should not modify items that are not alias. Also, could you add a test to check for example that a dict like {"line_color": "black"} goes through the dealiaser unchanged?

If the desired behaviour from above were not to happen (I don't really know how to check bokeh kwarg names though, sorry), we should also pass backend to the dealiaser so that None -> {}, backend="bokeh" -> unchanged_input and when none of the two conditions above are met, call matplotlib's normalize kwargs

[percygautam]

Sorry for the delay.
@OriolAbril There are two cases for kwargs, whether it is for matplotlib or it's for bokeh. If an alias(say 'lw') or full name( say 'linewidth') is passed to the normalized function, we'll get the dealiased name or same full name. Say a kwarg is passed that is not the param of matplotlib, it the normalized function will return it the same.
The error handling for wrong params is done at the backend.
I have modified test to show the desired change. (not valid param get passed on as it is)

[OriolAbril]

There have been some issues with merging

[OriolAbril]

kwargs should be dealiased before setting the defaults.

[percygautam]

I must have missed this one.

[OriolAbril]

I think these black blank lines are important for docs to render properly, could you check all inline example images are shown?

[percygautam]

Yes, there was some problem with blank lines. I checked all examples are working fine now.

[OriolAbril]

Thanks!

[OriolAbril]

I'll merge if tests pass