Update transforms for PIL deprecation #5898

kylematoba · 2022-04-27T11:27:49Z

Updates torchvision transformations to the call recommended at https://github.com/python-pillow/Pillow/blob/5d070222d21138d2ead002fd33fdf5adcb708941/src/PIL/Image.py#L93 (Pillow 9.1.0 release).

datumbox

Thanks for the contribution @kylematoba.

@pmeier @vfdev-5 Any thoughts on whether we should make the use of .Transform conditional on the PIL version? We probably need to investigate when this was introduced. Unless we just want to pump the minimum required PIL version.

kylematoba · 2022-04-27T13:54:16Z

It worked on Pillow 6.0.0 for me. I couldn't get my test to work on anything earlier for unrelated reasons (so your setup.py might be out of date).

pmeier

It worked on Pillow 6.0.0 for me.

I doubt that:

>>> import PIL.Image
>>> PIL.__version__
'9.0.1'
>>> PIL.Image.Transpose
AttributeError: module 'PIL.Image' has no attribute 'Transpose'

This was only introduced in python-pillow/Pillow#5954 which made it into the latest 9.1.0 release. I talked offline with @vfdev-5 about that some time ago. He wanted to pick it up, but I'm not aware of a PR regarding this yet.

Any thoughts on whether we should make the use of .Transform conditional on the PIL version? [...] Unless we just want to pump the minimum required PIL version.

I'm not opposed to that. Right now we require

vision/setup.py

Line 68 in f014841

pillow_ver = " >= 5.3.0, !=8.3.*"

and this was introduced in #3641. Bumping from >=5.3.0 to >=9.1.0 is quite a difference (~ 2.5 years), but it might still be worth it to avoid cluttering our code with version checks again that were removed in #3641. cc @NicolasHug

kylematoba · 2022-04-27T17:33:55Z

Oh yeah, you're right. My IDE was caching the original version of PIL. I thought it was weird 😂 .

I guess you can delay this until around the PIL hard change date of July 1, 2023:

DeprecationWarning: AFFINE is deprecated and will be removed in Pillow 10 (2023-07-01). Use Transform.AFFINE instead.

In another year, it seems reasonable to bump the dep to >= 9.0.0.

If you want, close this PR and I'll add a note to my calendar to look at this again in June 2023.

pmeier · 2022-04-28T07:33:08Z

I guess you can delay this until around the PIL hard change date of July 1, 2023

Having user facing warnings for something they didn't do is not desirable. So we should handle this before the next release.

If you are willing, please keep this PR open and finish it as soon as we decided which way to go:

If we decide to require Pillow>=9.1, you'll have to update setup.py and can keep the changes you already have.
If we decide not to update our PIL requirement, we will need to have something like

import PIL.Image

if tuple(int(part) for part in PIL.__version__.split(".")) >= (9, 1):
    FLIP_LEFT_RIGHT = PIL.Image.FLIP_LEFT_RIGHT
    ...
else:
    FLIP_LEFT_RIGHT = PIL.Image.Transpose.FLIP_LEFT_RIGHT
    ...

def kernel(img):
    ...
    img.transpose(FLIP_LEFT_RIGHT)
    ...

In case we need these constants in multiple modules, it probably makes sense to have a _pil_constants.py module and put the snippet above in there. With that you can do something like

from . import _pil_constants

def kernel(img):
    ...
    img.transpose(_pil_constants.FLIP_LEFT_RIGHT)
    ...

datumbox · 2022-04-28T08:01:43Z

@kylematoba Thanks for the additional info and for notifying us about this. :)

@pmeier Thanks for the analysis. Requiring users to be on the latest version of PIL is kind of a bummer. In principle I don't think it's a bad thing to bump the minimum version a bit but going from >=5.x to >=9.1 is quite a big jump. I don't know how many breaking changes and feature deprecations exist across 4 major versions and what effect this jump will have to users who make use of PIL directly on their software. Option 2 looks viable to me. Do we know how many constants we need to handle?

@NicolasHug I would love to get your thoughts on the above.

pmeier · 2022-04-28T08:44:43Z

going from >=5.x to >=9.1 is quite a big jump

I agree, but we have to do something or it will only get worse. We could do this incrementally, i.e. requiring one new major PIL version per release until we hit the version we are after, but that also feels weird since we don't actually care about the versions in between.

Do we know how many constants we need to handle?

I wrote a small script to check all deprecated constants and this is the result:

--------------------------------------------------------------------------------
Image.FLIP_LEFT_RIGHT
--------------------------------------------------------------------------------
torchvision/transforms/functional_pil.py::57
test/test_transforms.py::1527
--------------------------------------------------------------------------------
Image.FLIP_TOP_BOTTOM
--------------------------------------------------------------------------------
torchvision/transforms/functional_pil.py::65
test/test_transforms.py::1524
--------------------------------------------------------------------------------
Image.BILINEAR
--------------------------------------------------------------------------------
torchvision/transforms/functional_pil.py::243
test/test_transforms_tensor.py::781
test/test_onnx.py::420
--------------------------------------------------------------------------------
Image.NEAREST
--------------------------------------------------------------------------------
torchvision/transforms/functional_pil.py::317
torchvision/transforms/functional_pil.py::333
torchvision/transforms/functional.py::395
torchvision/transforms/functional.py::575
torchvision/transforms/functional.py::655
torchvision/transforms/functional.py::1015
torchvision/transforms/functional.py::1108
torchvision/transforms/transforms.py::301
torchvision/transforms/transforms.py::758
torchvision/transforms/transforms.py::872
torchvision/transforms/transforms.py::1271
torchvision/transforms/transforms.py::1392
test/test_transforms_tensor.py::780
--------------------------------------------------------------------------------
Image.AFFINE
--------------------------------------------------------------------------------
torchvision/transforms/functional_pil.py::326
test/test_transforms_tensor.py::775
--------------------------------------------------------------------------------
Image.BICUBIC
--------------------------------------------------------------------------------
torchvision/transforms/functional_pil.py::350
--------------------------------------------------------------------------------
Image.PERSPECTIVE
--------------------------------------------------------------------------------
torchvision/transforms/functional_pil.py::359
--------------------------------------------------------------------------------
Image.LINEAR
--------------------------------------------------------------------------------
test/test_transforms.py::176

We have usages in multiple places so IMO we should go for the extra module as explained in #5898 (comment) if we don't update our requirement.

NicolasHug · 2022-04-28T09:58:44Z

Re minimal supported version: PIL has a deprecation policy of 2 major versions, so if we want to be a good citizen, that means the minimum PIL version we should support is probably current version - 1?

Regardless, we'll have to support both ways (the deprecated one and the new one) until our minimum version is >= the version that removes the deprecated way. I don't see a better solution than the wrappers proposed above by Philip.

pmeier · 2022-04-28T13:01:24Z

minimum PIL version we should support is probably current version - 1?

Just to be clear: Given that the current version is 9.1.0 this means we can require pillow>=8, correct?

datumbox · 2022-04-28T13:16:42Z

@pmeier I'm OK with that. BTW we should investigate what's the best way to keep all dependencies in one place. Bumping PIL right now involves updating 100 places across CI, building scripts and configs.

kylematoba · 2022-04-29T08:14:15Z

Okay, I'll update the PR to make all instances of the deprecated constants contingent upon PIL.__version__.split(".")) >= (9, 1). Give me a few days.

kylematoba · 2022-04-30T10:39:59Z

I added transforms/_pil_constants.py which makes the constants contingent on the PIL version.

I extended my earlier changes to the docstrings, type annotations, and tests listed at #5898 (comment).

Note the slightly confusing rename from LINEAR to BILINEAR:
https://github.com/python-pillow/Pillow/blob/main/src/PIL/Image.py#L66
I kept the version-abstracted name as LINEAR (this is only used in a single test).

torchvision/transforms/functional.py

pmeier

Linting CI job is failing. Could you follow our instructions here to auto format the files you've edited? Otherwise LGTM. Thanks @kylematoba!

torchvision/transforms/functional.py

torchvision/transforms/_pil_constants.py

pmeier · 2022-05-02T06:31:41Z

torchvision/transforms/functional.py

@@ -572,7 +572,7 @@ def resized_crop(
            :class:`torchvision.transforms.InterpolationMode`.
            Default is ``InterpolationMode.BILINEAR``. If input is Tensor, only ``InterpolationMode.NEAREST``,
            ``InterpolationMode.BILINEAR`` and ``InterpolationMode.BICUBIC`` are supported.
-            For backward compatibility integer values (e.g. ``PIL.Image.NEAREST``) are still acceptable.
+            For backward compatibility integer values (e.g. ``PIL.Image(.Resampling).NEAREST``) are still acceptable.


I'm ok with this style in the docstring. Any objections?

() is fine, although as a nit, I think brackets [] are more common (typically in man pages)

@kylematoba Could you address this in all docstrings?

Sure, so replace instances of () with [] in my changes thusfar, and a second PR that just removes all of the lines about For backward compatibility integer values?

Sure, so replace instances of () with [] in my changes thusfar,

Yes.

and a second PR that just removes all of the lines about For backward compatibility integer values?

No. The docstring need to be changed indicating that passing int's is actually deprecated, e.g.

vision/torchvision/transforms/functional.py

Lines 1029 to 1031 in 6274080

.. warning::

This parameter was deprecated in ``0.12`` and will be removed in ``0.14``. Please use ``interpolation``

instead.

Furthermore, we need to change the warning

vision/torchvision/transforms/functional.py

Lines 415 to 419 in 6274080

if isinstance(interpolation, int):

warnings.warn(

"Argument interpolation should be of type InterpolationMode instead of int. "

"Please, use InterpolationMode enum."

)

to include the concrete deprecation date, e.g.

vision/torchvision/transforms/functional.py

Lines 1042 to 1045 in 6274080

warnings.warn(

"The parameter 'resample' is deprecated since 0.12 and will be removed 0.14. "

"Please use 'interpolation' instead."

)

For this we will be deprecating in 0.13 and remove in 0.15.

vmoens · 2022-05-04T09:15:40Z

A quick comment on the warning from a user perspective. I don't have the full context of this PIL deprecation issue so forgive me if there's anything I missed.

Every run of torchrl scripts we make gives us as many warnings as the number of processes that are importing torchvision (3 warnings / process x 32 process = almost 200 lines of warnings). This gives us long logs where it's hard to find what's going on.

Besides, correct me if I'm wrong, but it feels like it's a warning on which I have little agency -- it's not really that we're importing anything that will be deprecated: it's an issue in torchvision code, not ours. I think it's confusing for the users, as they're not fully informed of what's going on and who should fix what.

Apart from hiding those specific warnings (my last option is to do a temporary patch in torchrl to hide specific deprecation warnings, but i'm a bit reluctant to make a PR like this), is there anything that could be done?

pmeier · 2022-05-04T09:26:16Z

@vmoens

Apart from hiding those specific warnings, [...] is there anything that could be done?

The only other option is to force PIL < 9.1. After this PR there should be no warnings regardless of the PIL version. Unfortunately, until this patch will be included in a stable release will be 1-2 months. However, you could use the nightly build.

pmeier

I've checked CI and couldn't find a warning from PIL anywhere. Seems like we patched all places.

One small thing to do, otherwise LGTM. Thanks a lot @kylematoba!

pmeier · 2022-05-04T12:21:10Z

torchvision/transforms/functional.py

@@ -572,7 +572,7 @@ def resized_crop(
            :class:`torchvision.transforms.InterpolationMode`.
            Default is ``InterpolationMode.BILINEAR``. If input is Tensor, only ``InterpolationMode.NEAREST``,
            ``InterpolationMode.BILINEAR`` and ``InterpolationMode.BICUBIC`` are supported.
-            For backward compatibility integer values (e.g. ``PIL.Image.NEAREST``) are still acceptable.
+            For backward compatibility integer values (e.g. ``PIL.Image(.Resampling).NEAREST``) are still acceptable.


@kylematoba Could you address this in all docstrings?

torchvision/transforms/functional.py

NicolasHug

Thanks for the PR @kylematoba , minor comment below but LGTM

torchvision/transforms/_pil_constants.py

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>

github-actions · 2022-05-09T11:07:29Z

Hey @datumbox!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

Summary: * Update transforms for PIL deprecation * Changes agreed at #5898 * black, sort constants, version check * Format tests * Square brackets * Update torchvision/transforms/_pil_constants.py Reviewed By: YosuaMichael Differential Revision: D36281595 fbshipit-source-id: 677b0cb42dd23589197b757291d35a4b5f603c58 Co-authored-by: Nicolas Hug <contact@nicolas-hug.com> Co-authored-by: Philip Meier <github.pmeier@posteo.de> Co-authored-by: Nicolas Hug <contact@nicolas-hug.com> Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>

* Requested here #5898 (comment). * Fix tests * ufmt, not black Co-authored-by: Philip Meier <github.pmeier@posteo.de> Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>

Summary: * Requested here #5898 (comment). * Fix tests * ufmt, not black Reviewed By: NicolasHug Differential Revision: D36760927 fbshipit-source-id: 8ed8c6eaf32928f71ba6d4b3cf23a4b84f310d81 Co-authored-by: Philip Meier <github.pmeier@posteo.de> Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>

Update transforms for PIL deprecation

7874b06

facebook-github-bot added the cla signed label Apr 27, 2022

datumbox reviewed Apr 27, 2022

View reviewed changes

pmeier requested changes Apr 27, 2022

View reviewed changes

kylematoba added 2 commits April 30, 2022 10:50

Merge branch 'main' into main

80f8f60

Changes agreed at #5898

3cfe376

datumbox reviewed Apr 30, 2022

View reviewed changes

torchvision/transforms/functional.py Outdated Show resolved Hide resolved

pmeier reviewed May 2, 2022

View reviewed changes

kylematoba added 3 commits May 2, 2022 13:31

black, sort constants, version check

a74be25

Format tests

1ac68ca

Merge branch 'main' into main

cb13daa

pmeier approved these changes May 4, 2022

View reviewed changes

kylematoba added 2 commits May 5, 2022 13:37

Merge branch 'main' into main

57228df

Square brackets

77a2dfb

pmeier requested a review from NicolasHug May 5, 2022 12:33

NicolasHug approved these changes May 6, 2022

View reviewed changes

torchvision/transforms/_pil_constants.py Show resolved Hide resolved

pmeier and others added 2 commits May 6, 2022 11:46

Update torchvision/transforms/_pil_constants.py

3ee5542

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>

Merge branch 'main' into main

8597dd5

Merge branch 'main' into main

8743141

datumbox merged commit 423ddcd into pytorch:main May 9, 2022

datumbox added enhancement module: transforms dependency issue labels May 9, 2022

kylematoba mentioned this pull request May 9, 2022

Deprecate int as interpolation argument type #5974

Merged

rwightman mentioned this pull request Jun 10, 2022

PIL version check for enum change appears to break SIMD versions #6153

Closed

adamjstewart mentioned this pull request Jun 15, 2022

DeprecationWarning: BILINEAR is deprecated and will be removed in Pillow 10 Cadene/pretrained-models.pytorch#221

Open

pmeier mentioned this pull request Jan 11, 2023

[discussion] [wip] Move/adopt to torchvision frequent utils from detectron2 (and potentially other frameworks) #7070

Open

This was referenced Feb 9, 2023

Revert removal and deprecation of int values for interpolation? #7205

Closed

Undeprecate int constants for interpolation #7241

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update transforms for PIL deprecation #5898

Update transforms for PIL deprecation #5898

kylematoba commented Apr 27, 2022

datumbox left a comment

kylematoba commented Apr 27, 2022

pmeier left a comment

kylematoba commented Apr 27, 2022 •

edited

Loading

pmeier commented Apr 28, 2022

datumbox commented Apr 28, 2022

pmeier commented Apr 28, 2022 •

edited

Loading

NicolasHug commented Apr 28, 2022

pmeier commented Apr 28, 2022

datumbox commented Apr 28, 2022

kylematoba commented Apr 29, 2022 •

edited

Loading

kylematoba commented Apr 30, 2022

pmeier left a comment

pmeier May 2, 2022

NicolasHug May 4, 2022 •

edited

Loading

pmeier May 4, 2022

kylematoba May 5, 2022

pmeier May 5, 2022

vmoens commented May 4, 2022

pmeier commented May 4, 2022

pmeier left a comment

pmeier May 4, 2022

NicolasHug left a comment

github-actions bot commented May 9, 2022

	.. warning::
	This parameter was deprecated in ``0.12`` and will be removed in ``0.14``. Please use ``interpolation``
	instead.

	if isinstance(interpolation, int):
	warnings.warn(
	"Argument interpolation should be of type InterpolationMode instead of int. "
	"Please, use InterpolationMode enum."
	)

	warnings.warn(
	"The parameter 'resample' is deprecated since 0.12 and will be removed 0.14. "
	"Please use 'interpolation' instead."
	)

Update transforms for PIL deprecation #5898

Update transforms for PIL deprecation #5898

Conversation

kylematoba commented Apr 27, 2022

datumbox left a comment

Choose a reason for hiding this comment

kylematoba commented Apr 27, 2022

pmeier left a comment

Choose a reason for hiding this comment

kylematoba commented Apr 27, 2022 • edited Loading

pmeier commented Apr 28, 2022

datumbox commented Apr 28, 2022

pmeier commented Apr 28, 2022 • edited Loading

NicolasHug commented Apr 28, 2022

pmeier commented Apr 28, 2022

datumbox commented Apr 28, 2022

kylematoba commented Apr 29, 2022 • edited Loading

kylematoba commented Apr 30, 2022

pmeier left a comment

Choose a reason for hiding this comment

pmeier May 2, 2022

Choose a reason for hiding this comment

NicolasHug May 4, 2022 • edited Loading

Choose a reason for hiding this comment

pmeier May 4, 2022

Choose a reason for hiding this comment

kylematoba May 5, 2022

Choose a reason for hiding this comment

pmeier May 5, 2022

Choose a reason for hiding this comment

vmoens commented May 4, 2022

pmeier commented May 4, 2022

pmeier left a comment

Choose a reason for hiding this comment

pmeier May 4, 2022

Choose a reason for hiding this comment

NicolasHug left a comment

Choose a reason for hiding this comment

github-actions bot commented May 9, 2022

kylematoba commented Apr 27, 2022 •

edited

Loading

pmeier commented Apr 28, 2022 •

edited

Loading

kylematoba commented Apr 29, 2022 •

edited

Loading

NicolasHug May 4, 2022 •

edited

Loading