Handle argument "facecolors=None" correctly in plot_surface()

[ocastany]

PR Summary

Handle plot_surface(facecolors=None) correctly.
This patch avoids the error: UnboundLocalError: local variable 'color' referenced before assignment.

PR Checklist

Documentation and Tests

• [N/A] Has pytest style unit tests (and pytest passes)
• [N/A] Documentation is sphinx and numpydoc compliant (the docs should build without error).
• [N/A] New plotting related features are documented with examples.

Release Notes

• [N/A] New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
• [N/A] API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
• [N/A] Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

[rcomer]

Thank you for your contribution @ocastany. Could you provide an example that demonstrates the problem you are fixing?

[ocastany]

Here is an example showing the problem before the patch:

> from pylab import *
> fig = figure()
> ax = fig.add_subplot(projection='3d')
> X,Y = meshgrid(arange(5), arange(5))
> Z = sqrt(X**2 + Y**2)
> ax.plot_surface(X,Y,Z, facecolors=None)

UnboundLocalError: local variable 'color' referenced before assignment

[greglucas]

Could you add your example as a test so this doesn't come back again in the future?

[ocastany]

Sorry, I don't know where and how to "add my example as a test".

[melissawm]

Hi @ocastany - you can look at the test_axes3d.py file and add your example as a test that needs to pass. You can also look at https://matplotlib.org/stable/devel/testing.html for more details on writing and verifying tests for Matplotlib. Cheers!

[ocastany]

I added a test function in my GitHub repository. Can you use it?

[greglucas]

Yep, I can see your test and it looks good! Just a minor suggestion that we can do an image comparison that the facecolors=None explicit argument should do the same thing as the default. We have a test decorator to help with that which I've added as a suggestion.

[ocastany]

This looks good to me

[greglucas]

pre-commit.ci autofix

[greglucas]

Sorry @ocastany, it looks like the suggestions I had added additional white-space on some lines. precommit.ci can't autofix these because the PR is from your protected branch, so I suggested some additional changes to remove that which you will have to commit to your own branch.

[greglucas]

Thanks, @ocastany!