"c_was_none"? (otherwise the next line reads a bit weird)

Done.

would tend to merge the conditionals:

c = (facecolors if facecolors is not None     else "b" if classic_mode     else self._get_patches_for_fill.get_next_color())

Done.

Something funny is going on here (I know it was already there before, but still seems worth pointing out): we take x and y of any shapes and flatten them (they just need to have the same size), butc has to match either the shape ofx ory, not just the size.

In other words the following "work" (i.e. perform an implicit ravel()):

scatter(np.arange(12).reshape((3, 4)), np.arange(12).reshape((4, 3)), c=np.arange(12).reshape((3, 4)))scatter(np.arange(12).reshape((3, 4)), np.arange(12).reshape((4, 3)), c=np.arange(12).reshape((4, 3)))

but the following fail:

scatter(np.arange(12).reshape((3, 4)), np.arange(12).reshape((4, 3)), c=np.arange(12).reshape((6, 2)))# and evenscatter(np.arange(12).reshape((3, 4)), np.arange(12).reshape((4, 3)), c=np.arange(12))

Of course that last one has the best error message (irony intended):

ValueError: 'c' argument has 12 elements, which is not acceptable for use with 'x' with size 12, 'y' with size 12.

Well observed. I prefer not to change functionality as part of this refactoring, but I'm happy to open an issue/pr for that. - Also not sure what the intended behavior should be (should we modifyc to be as permissive as withx/y, or should we be more strict aboutx/y shape?

Edit: Issue added#12735

ValueError: 'c' argument has 12 elements, which is not acceptable for use with 'x' with size 12, 'y' with size 12.

is a really awful error. Has the intuitive functionality that was present before been restored?

Please continue this conversation at#12735. I think a PR that gave a better error for this case would be most welcome.

else: (goes together with theif: above)

Done.

Instead of tracking n_elem I think you can just donc=np.shape(c)[0] here?

You can have edge cases in whichnp.shape(c)[0] will raise here. E.g.

plt.scatter([1,2], [1,2], c='rgb')

...ValueError:During handling of the above exception, another exception occurred:~/dev/matplotlib/lib/matplotlib/axes/_axes.py in _parse_scatter_color_args(self, c, edgecolors, kwargs, xshape, yshape)   4144                         "acceptable for use with 'x' with size {xs}, "   4145                         "'y' with size {ys}."-> 4146                             .format(nc=np.shape(c)[0], xs=xsize, ys=ysize)   4147                     )   4148                 else:IndexError: tuple index out of range

Makes shure the intended message is printed, even if we cannot determine the shape of c. Even if it says -1 there it's better than just "tuple index out of range".

fair enough

I think the standard wording would be something like

"'c' must be either this or that, not\n{}".format(c)

(the newline being there only to handle cases of very long c's)

Message rewritten. I don't think that exception messages should contain explicit newlines.

I think I have a mild preference for moving the invalid shape error message up and just doraise ValueError(invalid_shape.format(...)) here and below instead of keeping thevalid_shape andn_elem tracking variables.

I feel that the overall structure is not very clear and that it can be written much more concise, but that's beyond this PR. Therefore I'd like to defer any further code changes within the function. As written in the original PR message:

I'm quite sure this function can be untangled more. This will become easier after this PR, because we can now add a further tests.

So the strategy is:

1. Get this PR in without changing more than neccesary of the original code.
2. Move the tests to directly testingAxes._parse_scatter_color_args, which is significantly faster than creating a figure each time.
3. This allows adding more tests an thus be surer that code changes do not modify the functionality.

At that point the only error we can get is the one thrown by to_rgba_array which I think is just fine to let through (Invalid RGBA argument: ...).

As above would like to defer this to future PRs.