Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make import PIL; PIL.Image work #6614

Closed
Kodiologist opened this issue Sep 23, 2022 · 16 comments
Closed

Make import PIL; PIL.Image work #6614

Kodiologist opened this issue Sep 23, 2022 · 16 comments

Comments

@Kodiologist
Copy link

Kodiologist commented Sep 23, 2022

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.

@hugovk
Copy link
Member

hugovk commented Sep 23, 2022

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

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

@Kodiologist
Copy link
Author

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

@aclark4life
Copy link
Member

aclark4life commented Sep 23, 2022

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.

@radarhere
Copy link
Member

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.

@Kodiologist
Copy link
Author

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.

@aclark4life aclark4life changed the title Make import PIL; PIL.Image work unconditionally Make import PIL; PIL.Image work Sep 24, 2022
@aclark4life
Copy link
Member

@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. 🤷

@radarhere
Copy link
Member

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.

@aclark4life
Copy link
Member

aclark4life commented Oct 6, 2022

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. 😄

@radarhere
Copy link
Member

radarhere commented Mar 6, 2023

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.

@tacaswell
Copy link
Contributor

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 #6614 (comment)) the simplest path? IF you don't want to always pay that import cost, then maybe look at something like scikit-image/scikit-image#5101 to get lazy imports?

@aclark4life
Copy link
Member

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.

@tacaswell
Copy link
Contributor

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.

@radarhere
Copy link
Member

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.

@aclark4life
Copy link
Member

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.

@radarhere
Copy link
Member

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

@nulano
Copy link
Contributor

nulano commented Apr 3, 2024

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'>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants