While you're at it: wouldn't it make sense to allow for the extent keyword to actually be passed to imshow instead of raising an error? As an application I could think of image comparisson where the image may be a photograph with units on the axes for which you'd want to check sparsity.

Actually, the current hard-coded implementation of extent yields unexpected results when using origin='lower'. The image flips, but the vertical scale does not:

fig, axs = plt.subplots(1, 2)
axs[0].spy(a)
axs[1].spy(a, origin='lower')

The fix is to not handle extent explicitly but let imshow do the work:

• This allows to use extent explicitly as @ImportanceOfBeingErnest proposed above
• Also this fixes the above unexpected behavior.

Why do we want to explicitly check for these kwargs? We silently ignore all other superfluous kwargs...

No I guess the idea is to pass all arguments to the underlying plot or imshow functions except those that would collide with the presets. For those an error is raised before the underlying function would raise a less understandable error.

I think one could as well pop them from the kwargs and issue a warning instead.

I’d be fine w a warning. But I’m not aware that we blacklist unused kwargs unless they conflict with another kwarg so I’d argue that should be changed.

Is this warn-on-unsupported **kwargs, but do not raise a TypeError, an agreed procedure? I have the feeling it's just something that nobody cared to check **kwargs so far.

IMO passing an invalid **kwargs is a fundamental programming error that should be detected and the source code adapted. Warnings may be overlooked. I'd opt for TypeError

If you pass an invalid kwarg to a function without **kwargs, this raises a TypeError. I think would be consistent to do the check so that **kwargs is not a wild-card, but just an abbreviation for for a defined set of parameter names.

I think this also depens on what is valid according to the docs.
If the docstring says "further arguments are passed to imshow", a warning would be a good choice, saying "interpolation is ignored and set to 'nearest'".
If the docstring says "you may supply all supported arguments to imshow, except 'interpolation'" raising an error would be justified.

Changed to just show a warning.

@dstansby I've fixed your requested change.

Is this warn-on-unsupported **kwargs, but do not raise a TypeError, an agreed procedure? I have the feeling it's just something that nobody cared to check **kwargs so far.

IMO passing an invalid **kwargs is a fundamental programming error that should be detected and the source code adapted. Warnings may be overlooked. I'd opt for TypeError

I agree that passing unused **kwargs should be an error - that's the topic of most of the recent work to convert to explicit keyword-only arguments instead of arbitrary kwargs.

So for what it's worth, I'd strongly recommend raising TypeError instead of a warning as was done in #11137, #11145, #11146, and #11271; this is one of the principles for Matplotlib 3.0!

@ImportanceOfBeingErnest @jklymak Is it ok if I switch back to error?

Would it be better to do kwargs.setdefault('linestyle', 'None')? That way if the user does pass it in it is respected (over-ruling spy's opinion of what the linestlye should be) and if they do not we get the opinionated default (which is different than the default of the underlying function).

Since my opinion was asked here: Yes raising an error should be fine as well; as commented already, the docs should then explicitely list the forbidden arguments.

A default linestyle argument does not make sense in my view. The reason is simply that you never want any linestyle for a spy plot. (In that sense the use of scatter might actually be more useful for spy, but I think that goes too far here.)

Moved to 3.1.

I agree with @ImportanceOfBeingErnest that a linestyle does not make sense in this context. This is a special purpose function and connecting the individual markers would be misleading at best.

Concerning warning vs. error: Will switch back to an error.

Agree with making conflicting kwargs an error as they are otherwise silently ignored.

@jklymak you approved the previous iteration where it was just a warning, are you still ok with the current design?

If there are conflicting arguments an error is fine. If there are unused arguments I’m still wary that this sets up the expectation that we will trap unused args elsewhere and we don’t.

Well, trapping unused arguments throughout the library is a bit a work in progress, isn't it?

I dunno, is that something we are trying to do? I'm not against it per-se if folks are willing to do it, it just seems like a heavy lift...

In my opinion, we should catch unused arguments. People think they achieve some effect when using these arguments. And we silently ignore their intent. „You can pass xxx but it does nothing.“ is usually not a good API.

Currently not doing this in all the places is no reason for not at least trying to move into the direction. After all, we currently have a mixed state.

#7728 for similar "warn-on-unused-kwarg" work.

thanks