Fix ToPIL

[bhashemian]

Fixes #1638

Description

Add ToPIL and ToPILD along with the unittests

Status

Ready/Work in progress/Hold

Types of changes

• Non-breaking change (fix or new feature that would not break existing functionality).
• Breaking change (fix or new feature that would cause existing functionality to change).
• New tests added to cover the changes.
• Integration tests passed locally by running ./runtests.sh --codeformat --coverage.
• Quick tests passed locally by running ./runtests.sh --quick.
• In-line docstrings updated.
• Documentation updated, tested make html command in the docs/ folder.

[bhashemian]

@Nic-Ma @wyli @rijobro
min tests are failing because now we have transformers that depends on Pillow and it is not installed by minimum requirements. Should I add pillow to requirements-min.txt or do you have other idea?

[rijobro]

How about changing this: PILImage, _ = optional_import("PIL.Image") to this: PILImage, has_pil = optional_import("PIL.Image").

And then this:

if isinstance(img, PILImage.Image):
    return img
if isinstance(img, torch.Tensor):
    img = img.detach().cpu().numpy()
return PILImage.fromarray(img)

to this:

if not has_pil:
    raise RuntimeError("ToPIL requires pillow to be installed")
if isinstance(img, PILImage.Image):
    return img
if isinstance(img, torch.Tensor):
    img = img.detach().cpu().numpy()
return PILImage.fromarray(img)

And then any tests can have the decorator added @skipUnless(has_pil, "PIL required").

Is that what you're looking for?

[rijobro]

Your tests would also need to use optional_import

[wyli]

@Nic-Ma @wyli @rijobro
min tests are failing because now we have transformers that depends on Pillow and it is not installed by minimum requirements. Should I add pillow to requirements-min.txt or do you have other idea?

thanks please add the relevant tests to this list: https://github.com/Project-MONAI/MONAI/blob/master/tests/min_tests.py#L29 so that they'll be skipped in the minimal test pipeline

[bhashemian]

Thank you @rijobro and @wyli!
I'll add the tests to be excluded but the problem with min.tests is not that. It fails before running any test.

Traceback (most recent call last):
38
  File "C:\hostedtoolcache\windows\Python\3.8.7\x64\lib\runpy.py", line 194, in _run_module_as_main
39
['monai.apps', 'monai.data', 'monai.engines', 'monai.handlers', 'monai.transforms', 'monai.visualize']
40
    return _run_code(code, main_globals, None,
41
  File "C:\hostedtoolcache\windows\Python\3.8.7\x64\lib\runpy.py", line 87, in _run_code
42
    exec(code, run_globals)
43
  File "D:\a\MONAI\MONAI\tests\min_tests.py", line 139, in <module>
44
    assert sorted(err_mod) == ["monai.engines", "monai.handlers"]
45
AssertionError

[bhashemian]

It fails at load_submodule, where more submodules have error than ["monai.engines", "monai.handlers]

MONAI/tests/min_tests.py

Line 135 in 9ae3005

_, err_mod = load_submodules(sys.modules["monai"], True)

[wyli]

It fails at load_submodule, where more submodules have error than ["monai.engines", "monai.handlers]

MONAI/tests/min_tests.py

Line 135 in 9ae3005

_, err_mod = load_submodules(sys.modules["monai"], True)

yes, optional_import is needed for 'import PIL' in the newly added unit tests as @rijobro mentioned

[wyli]

It fails at load_submodule, where more submodules have error than ["monai.engines", "monai.handlers]

MONAI/tests/min_tests.py

Line 135 in 9ae3005

_, err_mod = load_submodules(sys.modules["monai"], True)

yes, optional_import is needed for 'import PIL' in the newly added unit tests as @rijobro mentioned

sorry it's incorrect. perhaps it's because of this PILImage.Image type, not defined in the min. env https://github.com/Project-MONAI/MONAI/pull/1648/files#diff-24a052cc80d92db9eeddbd616450f309e9b88d293f187f469b9f73dbf4cf85f9R260

[bhashemian]

It fails at load_submodule, where more submodules have error than ["monai.engines", "monai.handlers]

MONAI/tests/min_tests.py

Line 135 in 9ae3005

_, err_mod = load_submodules(sys.modules["monai"], True)

yes, optional_import is needed for 'import PIL' in the newly added unit tests as @rijobro mentioned

sorry it's incorrect. perhaps it's because of this PILImage.Image type, not defined in the min. env https://github.com/Project-MONAI/MONAI/pull/1648/files#diff-24a052cc80d92db9eeddbd616450f309e9b88d293f187f469b9f73dbf4cf85f9R260

@wyli, sure, but how it can solve the issue with load_submodules? We already have a test that does not have optional import of PIL and things work fine:

MONAI/tests/test_pil_reader.py

Line 18 in 9ae3005

from PIL import Image

[wyli]

It fails at load_submodule, where more submodules have error than ["monai.engines", "monai.handlers]

MONAI/tests/min_tests.py

Line 135 in 9ae3005

_, err_mod = load_submodules(sys.modules["monai"], True)

yes, optional_import is needed for 'import PIL' in the newly added unit tests as @rijobro mentioned

sorry it's incorrect. perhaps it's because of this PILImage.Image type, not defined in the min. env https://github.com/Project-MONAI/MONAI/pull/1648/files#diff-24a052cc80d92db9eeddbd616450f309e9b88d293f187f469b9f73dbf4cf85f9R260

@wyli, sure, but how it can solve the issue with load_submodules? We already have a test that does not have optional import of PIL and things work fine:

MONAI/tests/test_pil_reader.py

Line 18 in 9ae3005

from PIL import Image

could remove all PILImage.Image type hints, add # type: ignore wherever needed

type hints with PILImage is fine, but PILImage.Image is not defined when pillow is not installed

[bhashemian]

It fails at load_submodule, where more submodules have error than ["monai.engines", "monai.handlers]

MONAI/tests/min_tests.py

Line 135 in 9ae3005

_, err_mod = load_submodules(sys.modules["monai"], True)

yes, optional_import is needed for 'import PIL' in the newly added unit tests as @rijobro mentioned

sorry it's incorrect. perhaps it's because of this PILImage.Image type, not defined in the min. env https://github.com/Project-MONAI/MONAI/pull/1648/files#diff-24a052cc80d92db9eeddbd616450f309e9b88d293f187f469b9f73dbf4cf85f9R260

@wyli, sure, but how it can solve the issue with load_submodules? We already have a test that does not have optional import of PIL and things work fine:

MONAI/tests/test_pil_reader.py

Line 18 in 9ae3005

from PIL import Image

could remove all PILImage.Image type hints, add # type: ignore wherever needed

type hints with PILImage is fine, but PILImage.Image is not defined when pillow is not installed

Exactly, that's the problem. Does anyone know a better solution for this? which is not particular to PIL. It's a pity to remove type checking for this. @Nic-Ma @rijobro @wyli

[bhashemian]

It fails at load_submodule, where more submodules have error than ["monai.engines", "monai.handlers]

MONAI/tests/min_tests.py

Line 135 in 9ae3005

_, err_mod = load_submodules(sys.modules["monai"], True)

yes, optional_import is needed for 'import PIL' in the newly added unit tests as @rijobro mentioned

sorry it's incorrect. perhaps it's because of this PILImage.Image type, not defined in the min. env https://github.com/Project-MONAI/MONAI/pull/1648/files#diff-24a052cc80d92db9eeddbd616450f309e9b88d293f187f469b9f73dbf4cf85f9R260

@wyli, sure, but how it can solve the issue with load_submodules? We already have a test that does not have optional import of PIL and things work fine:

MONAI/tests/test_pil_reader.py

Line 18 in 9ae3005

from PIL import Image

could remove all PILImage.Image type hints, add # type: ignore wherever needed
type hints with PILImage is fine, but PILImage.Image is not defined when pillow is not installed

Exactly, that's the problem. Does anyone know a better solution for this? which is not particular to PIL. It's a pity to remove type checking for this. @Nic-Ma @rijobro @wyli

I fixed it by keeping type checking as follow:

if TYPE_CHECKING:
    from PIL.Image import Image as PILImageImage
    from PIL.Image import fromarray as PILImageFromArray

    has_pil = True
else:
    PILImageFromArray, has_pil = optional_import("PIL.Image", name="fromarray")
    PILImageImage, _ = optional_import("PIL.Image", name="Image")