Pillow icon indicating copy to clipboard operation
Pillow copied to clipboard

Make `import PIL; PIL.Image` work

Open Kodiologist opened this issue 2 years ago β€’ 9 comments

With Pillow 9.2.0 and (e.g.) matplotlib 3.5.2:

$ python3 -c 'import PIL; PIL.Image'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
AttributeError: module 'PIL' has no attribute 'Image'
$ python3 -c 'import matplotlib, PIL; PIL.Image'
# No error.

This behavior is quite strange.

Kodiologist avatar Sep 23 '22 15:09 Kodiologist

Alternatively, maybe we should only allow the recommended from PIL import Image?

https://pillow.readthedocs.io/en/stable/installation.html#warnings

hugovk avatar Sep 23 '22 15:09 hugovk

Or import PIL.Image, or just import Pillow to eliminate confusion with https://pypi.org/project/PIL once and for all.

Kodiologist avatar Sep 23 '22 15:09 Kodiologist

I don't find this issue that strange, but yes, I guess I would deprecate import PIL.Image because ... I don't think I've ever typed that in 10+ years of Pillowing. So, if it helps eliminate some confusion, πŸ‘

Also we have been given control of PIL on PyPI, but we've not done anything with it. Enabling import Pillow is probably not an improvement... unless we print a message "You should probably use from PIL import Image."

So,

>>> import PIL.Image
Please import Image from PIL.

>>> import PIL
Please import Image from PIL.

>>> import Pillow
Please import Image from PIL.

Something like that.

aclark4life avatar Sep 23 '22 19:09 aclark4life

Just to point it out - since the removal of PILLOW_VERSION, which I think was the most backwards incompatible change that we've made, some people are still working around that or downgrading (although I don't know why). Even with a deprecation period, if we break the present version of matplotlib, torchvision again, and presumably other libraries, I expect there will be similar issues, and presumably another issue created like #6537.

I'm not opposed to removing PIL.Image for the sake of clarity, I just want to be able to tell users that we did consider what would happen, and this was the choice that was made anyway.

radarhere avatar Sep 24 '22 05:09 radarhere

I feel you. I maintain Hy, which isn't even 1.0 yet, and with every breaking change, however well-motivated, comes the wailing and gnashing of teeth.

Kodiologist avatar Sep 24 '22 11:09 Kodiologist

@radarhere I believe the most breaking change ever happened in Pillow 1.0 when import Image stopped working πŸ˜„

Anyway, again, I don't feel particularly inclined to add support to Pillow such that import PIL allows access to PIL.Image because ... wait for it ... Image is a module, not a package (directory containing other modules).

Correct me if I'm wrong, but to make PIL.Image available to reference via import PIL and without import PIL.Image we'd have to mkdir PIL/Image; mv PIL/Image.py PIL/Image/__init__.py ?

If so, I definitely don't see that happening. And if not, I'm not sure I see it happening without adding some workaround that would arguably be more confusing than just doing nothing. 🀷

aclark4life avatar Sep 24 '22 17:09 aclark4life

After experimenting, I'm not convinced that it's possible to tell from within Pillow if import PIL.Image or from PIL import Image was used, without either using traceback, which I expect would be imperfect and negatively impact speed, or restructuring Pillow (at least by renaming Image.py to _Image.py, filling Image.py with print("Please import Image from PIL."); from ._Image import *, adding from . import _Image as Image to __init__.py), which seems like overkill.

If I'm wrong, feel free to enlighten me.

radarhere avatar Oct 06 '22 12:10 radarhere

OP is asking for PIL.Image to be in namespace after import PIL which is not going to happen without:

mkdir PIL/Image
mv PIL/Image.py PIL/Image/__init__.py

Actually, the above alone doesn't help:

➜  Developer  mkdir PIL
➜  Developer  touch PIL/__init__.py
➜  Developer  mkdir PIL/Image
➜  Developer  touch PIL/Image/__init__.py
➜  Developer  python3
Python 3.10.6 (main, Aug 30 2022, 04:58:14) [Clang 13.1.6 (clang-1316.0.21.2.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import PIL
>>> PIL.__file__
'/Users/alexclark/Developer/PIL/__init__.py'
>>> PIL.Image
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 'PIL' has no attribute 'Image'

However, if PIL/Image/__init__.py contains from . import Image then it works:

Python 3.10.6 (main, Aug 30 2022, 04:58:14) [Clang 13.1.6 (clang-1316.0.21.2.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import PIL
>>> PIL.Image
<module 'PIL.Image' from '/Users/alexclark/Developer/PIL/Image/__init__.py'>

Which means maybe we could support this by adding from . import Image to PIL/__init__.py:

➜  Developer  mkdir PIL
➜  Developer  touch PIL/__init__.py
➜  Developer  touch PIL/Image.py
➜  Developer  vi PIL/__init__.py
➜  Developer  python3              
Python 3.10.6 (main, Aug 30 2022, 04:58:14) [Clang 13.1.6 (clang-1316.0.21.2.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import PIL
>>> PIL.Image
<module 'PIL.Image' from '/Users/alexclark/Developer/PIL/Image.py'>

Whether or not we should do this, I don't know. Current PIL __init__.py looks like this:

"""Pillow (Fork of the Python Imaging Library)

Pillow is the friendly PIL fork by Alex Clark and Contributors.
    https://github.com/python-pillow/Pillow/

Pillow is forked from PIL 1.1.7.

PIL is the Python Imaging Library by Fredrik Lundh and Contributors.
Copyright (c) 1999 by Secret Labs AB.

Use PIL.__version__ for this Pillow version.

;-)
"""

from . import _version

# VERSION was removed in Pillow 6.0.0.
# PILLOW_VERSION was removed in Pillow 9.0.0.
# Use __version__ instead.
__version__ = _version.__version__
del _version


_plugins = [
    "BlpImagePlugin",
    "BmpImagePlugin",
    "BufrStubImagePlugin",
    "CurImagePlugin",
    "DcxImagePlugin",
    "DdsImagePlugin",
    "EpsImagePlugin",
    "FitsImagePlugin",
    "FitsStubImagePlugin",
    "FliImagePlugin",
    "FpxImagePlugin",
    "FtexImagePlugin",
    "GbrImagePlugin",
    "GifImagePlugin",
    "GribStubImagePlugin",
    "Hdf5StubImagePlugin",
    "IcnsImagePlugin",
    "IcoImagePlugin",
    "ImImagePlugin",
    "ImtImagePlugin",
    "IptcImagePlugin",
    "JpegImagePlugin",
    "Jpeg2KImagePlugin",
    "McIdasImagePlugin",
    "MicImagePlugin",
    "MpegImagePlugin",
    "MpoImagePlugin",
    "MspImagePlugin",
    "PalmImagePlugin",
    "PcdImagePlugin",
    "PcxImagePlugin",
    "PdfImagePlugin",
    "PixarImagePlugin",
    "PngImagePlugin",
    "PpmImagePlugin",
    "PsdImagePlugin",
    "SgiImagePlugin",
    "SpiderImagePlugin",
    "SunImagePlugin",
    "TgaImagePlugin",
    "TiffImagePlugin",
    "WebPImagePlugin",
    "WmfImagePlugin",
    "XbmImagePlugin",
    "XpmImagePlugin",
    "XVThumbImagePlugin",
]


class UnidentifiedImageError(OSError):
    """
    Raised in :py:meth:`PIL.Image.open` if an image cannot be opened and identified.
    """

    pass

So I would say if adding from . import Image to PIL/__init__.py doesn't break any tests or cause any obvious harm, then maybe we do add it. But not in this upcoming release. πŸ˜„

aclark4life avatar Oct 06 '22 18:10 aclark4life

Trying the suggestion from the previous comment, I initially hit a circular import, but I was able to work around it.

So the following code does allow import PIL; PIL.Image to work.

diff --git a/src/PIL/Image.py b/src/PIL/Image.py
index cf9ab2df6..8692bd7b9 100644
--- a/src/PIL/Image.py
+++ b/src/PIL/Image.py
@@ -51,10 +51,8 @@ from . import (
     ExifTags,
     ImageMode,
     TiffTags,
-    UnidentifiedImageError,
-    __version__,
-    _plugins,
 )
+from ._version import __version__
 from ._binary import i32le, o32be, o32le
 from ._deprecate import deprecate
 from ._util import DeferredError, is_path
@@ -375,6 +373,8 @@ def init():
     if _initialized >= 2:
         return 0
 
+    from . import _plugins
+
     for plugin in _plugins:
         try:
             logger.debug("Importing %s", plugin)
@@ -3289,6 +3289,8 @@ def open(fp, mode="r", formats=None):
         fp.close()
     for message in accept_warnings:
         warnings.warn(message)
+    from . import UnidentifiedImageError
+
     msg = "cannot identify image file %r" % (filename if filename else fp)
     raise UnidentifiedImageError(msg)
 
diff --git a/src/PIL/__init__.py b/src/PIL/__init__.py
index 0e6f82092..31a2c41ed 100644
--- a/src/PIL/__init__.py
+++ b/src/PIL/__init__.py
@@ -13,6 +13,7 @@ Use PIL.__version__ for this Pillow version.
 ;-)
 """
 
+from . import Image
 from . import _version
 
 # VERSION was removed in Pillow 6.0.0.
diff --git a/src/PIL/_deprecate.py b/src/PIL/_deprecate.py
index fa6e1d00c..24523a023 100644
--- a/src/PIL/_deprecate.py
+++ b/src/PIL/_deprecate.py
@@ -2,7 +2,7 @@ from __future__ import annotations
 
 import warnings
 
-from . import __version__
+from ._version import __version__
 
 
 def deprecate(

However, it doesn't allow import PIL; PIL.ImageChops to work. So I'm concerned that a new issue could just be raised about that or another module.

radarhere avatar Mar 06 '23 11:03 radarhere

That import PIL.Image works is due to how Python's import mechanics work. I do not understand why you would go through the effort to break it.

If you want to avoid users having to import the sub-module isn't adding from . import Image in __init__.py (as suggested in https://github.com/python-pillow/Pillow/issues/6614#issuecomment-1270497974) the simplest path? IF you don't want to always pay that import cost, then maybe look at something like https://github.com/scikit-image/scikit-image/pull/5101 to get lazy imports?

tacaswell avatar Mar 18 '24 13:03 tacaswell

That import PIL.Image works is due to how Python's import mechanics work. I do not understand why you would go through the effort to break it.

Can you please clarify what "go through the effort to break it" means in this case? For example in 2.1.0 we broke import _imaging for a specific reason. I'm not sure if we went out of our way here to break anything here… yet.

aclark4life avatar Mar 18 '24 14:03 aclark4life

guess I would deprecate import PIL.Image because ... I don't think I've ever typed that in 10+ years of Pillowing. So, if it helps eliminate some confusion, πŸ‘

Which I read as "eventually break import PIL.Image"

At the end of the day you have:

PIL/
   __init__.py
   Image.py
   ...
   bunch.py
   of.other
   files.py

To make import PIL.Image not work you are going to have to do something funny which will break down stream users and (in my view) be orders of magnitude more confusing. Libraries should be consistent with how Python actually works, not an incorrect mental model of confused users....


diff --git a/src/PIL/__init__.py b/src/PIL/__init__.py
index 63a45769b..85b26f25c 100644
--- a/src/PIL/__init__.py
+++ b/src/PIL/__init__.py
@@ -84,3 +84,9 @@ class UnidentifiedImageError(OSError):
     """

     pass
+
+
+try:
+    from . import Image  # noqa, must be last because it uses _plugins
+except ImportError:
+    ...

is enough to make the OP's request work. It has to be at the end (and ignore the QA for "imports at the top" because of _plugins and in the try-except is because in setup.py this file gets exec'd before PIL._imaging has been built so ignore the import error (which should probably be fixed by not execing as part of the build process)). I agree with @radarhere that if you do this for Image you may get this for what ever other modules so I would either do nothing (other than maybe writing it as from PIL import Image in your docs as a convention) or going all-in on lazy import of everything.

tacaswell avatar Mar 18 '24 19:03 tacaswell

So, the code from the original post is about an interaction between matplotlib and Pillow. A matplotlib maintainer has weighed in on this and said that the behaviour is fine. So perhaps we aren't concerned about the way our two libraries relate.

Setting that aside then, my thoughts are

  1. Should 'PIL.Image' be deprecated? Let's not go out of our way to break functionality, since we value backwards compatibility. My hope would be that anyone who is confused about this will visit our documentation and discover that we exclusively refer to from PIL import Image.
  2. Should 'PIL.Image' be actively supported? I don't think changing the import structure of our library is minor, and in the absence of a solid reason, I don't think we need to. If import PIL; PIL.Image is unreliable, that's ok, as it is not recommended behaviour.

I'm inclined to close this.

radarhere avatar Apr 03 '24 12:04 radarhere

Close-away, however I don't recall why import PIL.Image works when matplotlib is imported first? And yeah, we shouldn't go out of our way to break import PIL.Image because it happens to work in some scenario somewhere for some reason. We'd only do it if we felt like we had reason to do it within Pillow for Pillow's sake or for some much more major compatibility issue.

aclark4life avatar Apr 03 '24 13:04 aclark4life

why import PIL.Image works when matplotlib is imported first?

It's not that import PIL.Image works differently, it's that PIL.Image works after importing matplotlib. It would be because matplotlib internally runs import PIL.Image, so

import matplotlib, PIL; PIL.Image

is really

import PIL.Image, PIL; PIL.Image

radarhere avatar Apr 03 '24 13:04 radarhere

import PIL.Image, PIL; PIL.Image

It's not even necessarily that, since

>>> from PIL import Image
>>> import PIL
>>> PIL.Image
<module 'PIL.Image' from 'C:\\Users\\Nulano\\AppData\\Roaming\\Python\\Python311\\site-packages\\PIL\\Image.py'>

nulano avatar Apr 03 '24 18:04 nulano