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.

Sign up for GitHub

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

Jump to bottom

Raise a TypeError when Image() is called #7461

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Raise a TypeError when Image() is called #7461

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ae96f81
Raise a TypeError when Image() is called
radarhere Oct 13, 2023
54792a4
Use new() instead of Image()._new()
radarhere Oct 13, 2023
50a5bc8
Pass core image object to _init()
radarhere Oct 13, 2023
d29383b
Merge branch 'main' into init
radarhere Oct 17, 2023
1e16747
Merge branch 'main' into init
radarhere Jan 31, 2024
a2f169b
Merge branch 'main' into init
radarhere Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions Tests/test_image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ def test_sanity(self):
# with pytest.raises(MemoryError):
# Image.new("L", (1000000, 1000000))

def test_direct(self):
with pytest.raises(TypeError):
Image.Image()

def test_repr_pretty(self):
class Pretty:
def text(self, text):
Expand Down
51 changes: 31 additions & 20 deletions src/PIL/Image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,16 +481,11 @@ class Image:
_close_exclusive_fp_after_loading = True

def __init__(self):
# FIXME: take "new" parameters / other image?
# FIXME: turn mode and size into delegating properties?
self.im = None
self._mode = ""
self._size = (0, 0)
self.palette = None
self.info = {}
self.readonly = 0
self.pyaccess = None
self._exif = None
msg = (
"Images should not be instantiated directly. "
"Use the module new() function instead."
)
raise TypeError(msg)

@property
def width(self):
Expand All @@ -508,8 +503,24 @@ def size(self):
def mode(self):
return self._mode

def _prepare(self):
self.im = None
self._mode = ""
self._size = (0, 0)
self.palette = None
self.info = {}
self.readonly = 0
self.pyaccess = None
self._exif = None

@classmethod
def _init(cls):
self = cls.__new__(cls)
self._prepare()
return self

def _new(self, im):
new = Image()
new = Image._init()
new.im = im
new._mode = im.mode
new._size = im.size
Expand Down Expand Up @@ -695,7 +706,7 @@ def __getstate__(self):
return [self.info, self.mode, self.size, self.getpalette(), im_data]

def __setstate__(self, state):
Image.__init__(self)
self._prepare()
info, mode, size, palette, data = state
self.info = info
self._mode = mode
Expand Down Expand Up @@ -980,7 +991,7 @@ def convert_transparency(m, v):
else:
# get the new transparency color.
# use existing conversions
trns_im = Image()._new(core.new(self.mode, (1, 1)))
trns_im = Image._init()._new(core.new(self.mode, (1, 1)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image._init()._new() is quite common and very ugly. In this case we don't want to create a new image second time just to copy empty info and palette properties.

I suggest following:

im._new(core.new()) — creates image like im, copying info and palette. I'm not sure how often this method is used this way.
Image._init(core.new()) — creates empty image using only core image object.

The only place I see where we need to create empty Image object (without core object) is new() function, but it could be easily avoided.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've cherry-picked #7460 here, since that helps.

I've pushed a commit with a version of your suggestion. It's not exact, because it turns out that _new() doesn't just copy existing palettes, it also creates new ones. It's not enough to just attach a palette afterwards in new(), because something like Image.linear_gradient("P") also needs a palette.

if self.mode == "P":
trns_im.putpalette(self.palette)
if isinstance(t, tuple):
Expand Down Expand Up @@ -2874,7 +2885,7 @@ class ImageTransformHandler:
def _wedge():
"""Create greyscale wedge (for debugging only)"""

return Image()._new(core.wedge("L"))
return Image._init()._new(core.wedge("L"))


def _check_size(size):
Expand Down Expand Up @@ -2918,7 +2929,7 @@ def new(mode, size, color=0):

if color is None:
# don't initialize
return Image()._new(core.new(mode, size))
return Image._init()._new(core.new(mode, size))

if isinstance(color, str):
# css3-style specifier
Expand All @@ -2927,7 +2938,7 @@ def new(mode, size, color=0):

color = ImageColor.getcolor(color, mode)

im = Image()
im = Image._init()
if mode == "P" and isinstance(color, (list, tuple)) and len(color) in [3, 4]:
# RGB or RGBA value for a P image
from . import ImagePalette
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

    palette = None
    if mode == "P" and isinstance(color, (list, tuple)) and len(color) in [3, 4]:
        # RGB or RGBA value for a P image
        from . import ImagePalette

        palette = ImagePalette.ImagePalette()
        color = palette.getcolor(color)
    im = Image._init(core.fill(mode, size, color))
    im.palette = palette
    return im

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per my last comment, _init(im) needs to create a palette for P mode images, which might lead to two ImagePalettes being created here.

However, we don't need getcolor() to know what the color index should be - it is always going to be zero, as this is the first color in the palette. Taking advantage of that, we can just re-use the palette from _init().

Expand Down Expand Up @@ -3547,7 +3558,7 @@ def effect_mandelbrot(size, extent, quality):
(x0, y0, x1, y1).
:param quality: Quality.
"""
return Image()._new(core.effect_mandelbrot(size, extent, quality))
return Image._init()._new(core.effect_mandelbrot(size, extent, quality))


def effect_noise(size, sigma):
Expand All @@ -3558,7 +3569,7 @@ def effect_noise(size, sigma):
(width, height).
:param sigma: Standard deviation of noise.
"""
return Image()._new(core.effect_noise(size, sigma))
return Image._init()._new(core.effect_noise(size, sigma))


def linear_gradient(mode):
Expand All @@ -3567,7 +3578,7 @@ def linear_gradient(mode):

:param mode: Input mode.
"""
return Image()._new(core.linear_gradient(mode))
return Image._init()._new(core.linear_gradient(mode))


def radial_gradient(mode):
Expand All @@ -3576,7 +3587,7 @@ def radial_gradient(mode):

:param mode: Input mode.
"""
return Image()._new(core.radial_gradient(mode))
return Image._init()._new(core.radial_gradient(mode))


# --------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion src/PIL/ImageFile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class ImageFile(Image.Image):
"""Base class for image file format handlers."""

def __init__(self, fp=None, filename=None):
super().__init__()
super()._prepare()

self._min_frame = 0

Expand Down