Indeed, hunting down this bug was an adventure! For SunPy, we do some fun stuff with remapping data to a different view, e.g.:

We had an implementation that used pcolormesh(), but we recently added an alternative option to try imshow() instead. That means supplying imshow() with a nonaffine transform, which meant that it was affected by the bug fixed in this PR. There were peculiar differences between the outputs of the two approaches, but always no larger than a display pixel, so I wasn't sure whether I was crazy or not. Because it's at the display-pixel level, you can't simply zoom in interactively on a plot to see the discrepancy more clearly. It was only after I found that imshow() with just transData and with a nonaffine identity transform composited with transData gave different results that I started peeling through all of the layers in matplotlib to find the bug.

I added a stub file in #30058 to have the mypy errors go away.

It is slightly preferred if you can create a test using the public API (not accessing the _image module directly), but I leave it up to you if you rewrite the tests or wait for #30058. Otherwise looks good! Nice catch!

It is slightly preferred if you can create a test using the public API (not accessing the _image module directly)

Heh, I was hesitant to access _image in a test until I noticed that an existing test already did so, which is why I put this new test right below that one. I think it is prudent to keep a test that accesses _image module directly, given that it is a C extension and it's helpful to know if something breaks in the extension rather than in the Python above it.

There are a bunch of layers of private API between _image.resample() and public API: _image.resample() -> image._resample() -> image._ImageBase._make_image() -> image.*Base.make_image(). So, not only would the test be a little awkward to write, a future developer would have to repeat what I did in peeling back all of the Python layers before realizing that the relevant code is actually in the C extension.

(Edit: As realized down below, _image.resample() is actually already imported as image.resample(), so it is straightforward to convert the test to public API.)

I'll point out that this PR implicitly already has a test using public API, given that baseline images had to be updated. The plotting of an image using a log scale uses a nonaffine transform, and hence was affected by the bug.

Fair enough. As I said, it is preferred but not strictly required. More that is was worth asking, especially with the mypy errors (for which my solution did not work btw, but the one provided above should).

@ksunden Thanks for pointing out that image has from matplotlib._image import *! Thus, not only are the constants imported into the image namespace, but so is _image.resample() itself. So, I have adjusted the test to use entirely public API.

I can mostly understand the line plots, but do you have an example of how this changes in an image itself? Possibly one that is rather low resolution so it is clear how the changes affect it. The test image that did change is a bit too high resolution to see clearly, I think.

As I mentioned above, it's a little challenging to show the behavior in an obvious manner because it is typically only at the display-pixel level. That means you can't see the discrepancies better through zooming in an interactive plot or plotting at a larger DPI. Perhaps there is some way I am not aware of, but I take a different approach below.

Here's a comparison script to generate images for comparison (without the fix in this PR):

import numpy as np

import matplotlib.pyplot as plt
from matplotlib.transforms import Transform

data = np.arange(12321).reshape((111, 111)) % 4

class NonAffineIdentityTransform(Transform):
    input_dims = 2
    output_dims = 2

    def inverted(self):
        return self

fig = plt.figure()
ax = fig.add_subplot()
ax.imshow(data, extent=[0, 111, 0, 111], transform=ax.transData)
ax.set_title('affine')
fig.savefig('affine.png')

fig = plt.figure()
ax = fig.add_subplot()
ax.imshow(data, extent=[0, 111, 0, 111], transform=NonAffineIdentityTransform() + ax.transData)
ax.set_title('nonaffine')
fig.savefig('nonaffine.png')

At normal display, it's difficult to discern any difference between the two images:

If I then use an image editor to zoom in to the origin:

affine:

nonaffine:

The data array was specifically chosen so that 3 data pixels map to 10 display pixels, just as with the line plots above. You can see that between the affine image and the nonaffine image, the 3-4-3 pattern of display pixels has shifted. (That is, a data pixel that used to span 3 display pixels now spans 4 display pixels, and the data pixel right after it has changed from 4 display pixels to 3 display pixels.) It is difficult to know which one is "right" from these images, given all of the additional stuff that is done to render the image, but that's why the test in this PR checks the output of the low-level resample().

Is any further action needed from me for this PR to be merged? Thanks!

All green again