Just pushed up the fix to the pep8 error. Everything else passes, though. We probably should make sure all of the aliases I made make sense. It might even make sense to have a single location with this information so that it can be used as a default, and also used by the prop_cycle validator.

Buried someplace in Artist isn't there a way to ask the artists what their aliases are?

Yes, and that mechanism should be burned with fire.
On Apr 28, 2016 9:54 PM, "Thomas A Caswell" notifications@github.com
wrote:

Buried someplace in Artist isn't there a way to ask the artists what
their aliases are?

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#6345 (comment)

I am 👍 on merging this as-is and worrying about cleaning up the repeated content / centralizing the location of the alias mappings later. I think that this is something that traitlets may provide a reasonable programatic access to.

@WeatherGod Are you happy with the normalize_kwarg api?

Eh, like I said, it might make more sense to break it out into parts
because it does more than just normalize. But since this is already in
v1.5, the point is moot. One thing that is concerning is that I don't see
any unit tests for it, so how do we know the precedence-handling works
correctly? For example, for an alias map that has two property keys that
share a property value in their precedence list, and that property is
provided as a kwarg. Or a property key that exists in another property's
alias map (thinking of color being in the markerfacecolor's precedence list.

But those concerns aren't about the API itself. I do think the precedence
list is backwards (I would expect highest precedence first), but that's
just me.

On Sun, May 1, 2016 at 9:59 PM, Thomas A Caswell notifications@github.com
wrote:

@WeatherGod https://github.com/WeatherGod Are you happy with the
normalize_kwarg api?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#6345 (comment)

The tests are at https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/tests/test_cbook.py#L347

I do not thing that the normalize step should deal with the fall backs for facecolor vs color. The intent of this function was to normalize out getting both color and c (just the aliasing), the rest of (required, forbidden, and allowed) are there to make **kwargs a bit safer to work with.

I moved the alias map dictionary up to a module-level private dictionary.

This passes now.

I am still 👍 on this. It makes things better (but obviously not perfect and applied everywhere).

Leave to @efiring to merge.

@tacaswell Should I go ahead and merge v1.5.x into v2.x, and v2.x into master now, or do you want to do that sweep now or later? Do you expect those merges to be completely straightforward?

If you want to do it go ahead. We should have a better policy of when to do that.

I think there is a conflict between 2.x and master at the moment.

On 2016/05/04 8:51 AM, Thomas A Caswell wrote:

If you want to do it go ahead. We should have a better policy of when to
do that.

I think there is a conflict between 2.x and master at the moment.

I did both merges and pushed; I didn't see any problems or unexpected
changes.

Failure on 2.x! It generates quite a different test image. Here is the expected image, from 1.5.x:

And here is what the merge is producing on 2.x:

@WeatherGod, sorry, but can you look at this? Something rather fundamental seems to have gone wrong here, and I don't have time to look at it right now.

The color change is expected. If the property cycle does not contain color it falls back to black.

But why did the order of the line widths change?

off the top of my head, my guess is that the cycler was getting advanced internally, but its values not used until the last two lines. Makes me think the fallback handling to black is not correct?

The test code is below. If the cycler is supposed to advance with each call to plot, then v2.x is doing the right thing, and it is 1.5.x that is incorrect. The test is calling plot 3 times with a cycle of 2 values, and with lw overriding the cycle. Then there are two calls with no override, so we get linewidths of 4 and 2, in that order.

I think the thing to do is to change the expected plot for v2.x and master, and not worry about why the behavior is different in 1.5.x, since we hope we don't have to release that, and this is a corner case with a new feature anyway.

But is this correct? Is the explicit 'lw' overriding the 'linewidth' because it is explicit, or because short form overrides long form, or is it potentially random? Do we need more tests and illustrations of how the combination of the cycler and alias handling are supposed to work?

@image_comparison(baseline_images=['property_collision_plot'],
                  remove_text=True, extensions=['png'])
def test_property_collision_plot():
    fig, ax = plt.subplots()
    ax.set_prop_cycle('linewidth', [2, 4])
    for c in range(1, 4):
        ax.plot(np.arange(10), c * np.arange(10), lw=0.1)
    ax.plot(np.arange(10), 4 * np.arange(10))
    ax.plot(np.arange(10), 5 * np.arange(10))

The test code is below. If the cycler is supposed to advance with each call to plot, then v2.x is doing
the right thing, and it is 1.5.x that is incorrect. The test is calling plot 3 times with a cycle of 2 values, and with lw overriding the cycle. Then there are two calls with no override, so we get linewidths of 4 and 2, in that order.

No, the first three calls to ax.plot() should not advance the cycler.

I doubt this has anything to do with alias handling. Rather, the logic in prop_cycler is such that it only advances the property cycler if it has properties that weren't specified by the user. I think the recent fallback handling code broke this implicit assumption by inserting a color property into the property cycle (sort of, I think -- I haven't looked closely).

The way the fall back is implemented is to mulitply any cycler with out 'color' by cycler('color', 'k') iirc

I have a fix for this on #6374

I can pick it apart add do the last 2 commits as thier own PR, I was just being lazy and trying to avoid have to do a re-base.