A couple of suggestions:

• say that this "closes 2-D array RGB and RGBA values not understood in plt.plot() #18411" and then it will be automatically closed...
• have a bit more description in the PR. I appreciate you describe the problem in the Issue, but its nicer not to have to click around
• can you add a test? Otherwise next time this all gets optimized by someone, they will forget this case, and it will need to be fixed again.

A couple of suggestions:

* say that this "closes #18411" and then it will be automatically closed...

* have a bit more description in the PR. I appreciate you describe the problem in the Issue, but its nicer not to have to click around

* can you add a test?  Otherwise next time this all gets optimized by someone, they will forget this case, and it will need to be fixed again.

I am currently trying to create a test. The issue is as follows: The error is produced only when plt.show() is executed. However, it seems that plt.show() never gets executed in any of the existing tests. How would I check for the error when not using plt.show() ?

fig.canvas.draw() will force the rendering.

fig.canvas.draw() will force the rendering.

Thank you, it is working great. Currently I am testing for

3*, 4*, 1D-color-array
13, 14 2D-color-array

on plot() and scatter() functions. Which other cases do you suppose should be included in the test? I was thinking about hist() for example.

Using https://matplotlib.org/3.3.0/api/testing_api.html#matplotlib.testing.decorators.check_figures_equal may be very useful here so you can pass the ndim==1 and ndim==2 versions of the same color to the same function and then assert that they generate identical output.

@danuo overall this looks good - be sure to ping us for review if it gets buried...

I have all the tests fully working with the check_figures_equal decorator. All tests fail with current release and all pass with my commit. Would test_axes.py be the right place for these tests? One test looks like this

@check_figures_equal()
def test_2dcolor_plot(fig_test, fig_ref):
    color = np.array([0.1,0.2,0.3])
    fig_test.subplots().plot([1,2],[1,2],c=color.reshape((-1)))
    fig_ref.subplots().plot([1,2],[1,2], c=color.reshape((1,-1)))

test_axes is a reasonable place. Maybe merge them all into one figure with N axes instead of N individual tests (should run marginally faster)

Probably worth also adding a test in test_colors of the conversion function.

I have a slight concern that we are going to find a different case where projecting down here is going to give the wrong answer, but apparently we don't have any test that exercises this code path at all...

@tacaswell

Probably worth also adding a test in test_colors of the conversion function.

I am not sure what you mean by this. I have implemented all of your other proposals

@danuo this needs two reviews, so be sure to ping if you don't get a second one. @tacaswell may or may not clarify what he meant 😉

Putting a test in test_colors

assert mcolors.to_rbga([r, g, b]) == mcolors.to_rgba([[r, g, b]])

or similar to directly test the change (rather than implicitly via the plot / scatter behavior).

Putting a test in test_colors

assert mcolors.to_rbga([r, g, b]) == mcolors.to_rgba([[r, g, b]])

or similar to directly test the change (rather than implicitly via the plot / scatter behavior).

@tacaswell Done, I have implemented such a test in test_colors. Again, the test fails with current matplotlib version and succeeds with this fix. Can you make a second review?

This will need a rebase due to the conflicts.

Thanks @danuo! Congratulations on your first PR to Matplotlib 🎉 We hope to hear from you again.