Return type for PIL.Image.load

[DMRobertson]

A first step towards #9126.

Points I wasn't sure about:

• is this well-typed enough to be useful?
• does the Union PixelAccess | PyAccess return value actually get us anything? E.g. perhaps a Protocol describing the common parts of these two would be more useful

[Avasam]

(I am not a maintainer, I just contribute a lot here)

1. Incomplete types are always better than flat-out wrong ones. Typeshed also generally prefers false-negatives over false-positive. So even def load(self): ... or def load(self) -> Incomplete: ... would be better than a wrong type.

2. We tend to avoid unions in return types (see https://github.com/python/typeshed/blob/main/CONTRIBUTING.md#conventions and Document why having a union return type is often a problem mypy#1693).
   In your case you can't use an overload and you can't typeguard based on an ImportError. I think a protocol may still have variance issue (trying to assign AccessProtocol to a PyAccess for instance).
   There's a proposal that aims at solving this kind of problem: AnyOf - Union for return types typing#566

[Avasam]

If a solution is found for the return type. Do you think you could do all the other def load as well? Since they seem to also return the result of ImageFile.ImageFile.load or Image.Image.load?

[JelleZijlstra]

@DMRobertson could you respond to Avasam's feedback?

[DMRobertson]

Apologies all for the delay in responding. I have some spare time now; let me see if I can fix this up.

[DMRobertson]

Taking a look at this with fresher eyes, I think(?) the history here is:

• Image.load originally returned a PixelAccess object, the latter defined as a CPython extension only
• In order to support running this on PyPy another implementation called PyAccess was written using cffi. (Use CFFI for the pixel access object python-pillow/Pillow#248 and Cffi pixelaccess python-pillow/Pillow#476). As the docs for PyAccess put it:

  The PyAccess module provides a CFFI/Python implementation of the PixelAccess Class. This implementation is far faster on PyPy than the PixelAccess version.

So I think these really are meant to be implementations of the same interface. For that reason, I think returning a protocol rather than ImplA | ImplB best captures the intent of the library.

2. I think a protocol may still have variance issue (trying to assign AccessProtocol to a PyAccess for instance).

Similarly, I think the PIL authors don't really intend for any downstream user to care about the distinction between PixelAccess and PyAccess, so I don't think what you describe is likely to be a problem in practice. (Though let's see what the primer says.)

If a solution is found for the return type. Do you think you could do all the other def load as well? Since they seem to also return the result of ImageFile.ImageFile.load or Image.Image.load?

Sure, I can add that.

[Akuli]

I think returning ImplA | ImplB is probably the best solution. If we add a protocol, it becomes difficult for users to use it in type annotations, but PixelAccess | PyAccess isn't too bad although it isn't great either.

A protocol can be closer to what PIL authors intended, but typeshed generally types things with a "practicality beats purity" mindset, and IMO adding a fictional protocol wouldn't be very practical.

[github-actions]

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

[Akuli]

While I don't like the fictional protocol, this is a big improvement over -> None and can be improved in future PRs.