Feature/sg 000 no explicit pil

[BloodAxe]

Looks like this PR fixes the problem with PIL in Colab and users having to restart the runtime
https://colab.research.google.com/drive/1XpTQvHAPvQRhLFRpCpwoVzrNcYNT72yE#scrollTo=0IIEfRas22hg

[dagshub]

Join the discussion on DagsHub!

[shaydeci]

Looks like this PR fixes the problem with PIL in Colab and users having to restart the runtime
https://colab.research.google.com/drive/1XpTQvHAPvQRhLFRpCpwoVzrNcYNT72yE#scrollTo=0IIEfRas22hg

But there are multiple places in our code which we use PIL....I am a bit concerned about this since we dont have an upper limit on torchvision version (assuming thats where we get our pil version from)
Maybe we should fix to the pil version from the torchvision version installed ?

Dont know whats worse TBH, maybe we should consult with @Shani-Perl , I also think crashing notebooks are a disaster.

[BloodAxe]

There are indeed places where we use Pillow. However you're pointed earlier right that torchvision also has PIL dependency in it, so we still get Pillow installed. My understanding why removing Pillow from our requirements helps - torchvision has Pillow version requirements that are different, free from this bug.
If we inspect the environment with this fix, we will see that installed Pillow is Pillow==8.4.0. SG requires pillow>=9.2.0 which is of much higher version.

[shaydeci]

There are indeed places where we use Pillow. However you're pointed earlier right that torchvision also has PIL dependency in it, so we still get Pillow installed. My understanding why removing Pillow from our requirements helps - torchvision has Pillow version requirements that are different, free from this bug.
If we inspect the environment with this fix, we will see that installed Pillow is Pillow==8.4.0. SG requires pillow>=9.2.0 which is of much higher version.

Yes, but the 9.2.0 might have been an overkill. I looked at @tomerkeren42 commit message from when we updated the requirement in #417:

Pillow version was upgraded from 6.2.0 to 9.2.0, because of following bug was found while using pillow 8.2.0: Using transformations of random scale, which can be lower than expected crop-size, and then pad short size could lead to a bug in pillow version of padding - it was using a drawing rectangle (of zeros) instead of new method in newer versions. This method could create the following bug: self.draw.draw_rectangle(xy, ink, 0, width) TypeError: an integer is required (got type tuple)

I feel a bit more comfortable fixing it to 8.4.0, after checking we dont get this (actually, I think predict uses it internally). WDYT?

[BloodAxe]

torchvision uses this range of versions:

# Excluding 8.3.* because of https://github.com/pytorch/vision/issues/4934
pillow_ver = " >= 5.3.0, !=8.3.*"
pillow_req = "pillow-simd" if get_dist("pillow-simd") is not None else "pillow"
requirements.append(pillow_req + pillow_ver)

Ok, let's try this. What about we go with Pillow>8.3,=8.* - E.g limiting to 8.* but newer than 8.3?

[shaydeci]

torchvision uses this range of versions:

# Excluding 8.3.* because of https://github.com/pytorch/vision/issues/4934
pillow_ver = " >= 5.3.0, !=8.3.*"
pillow_req = "pillow-simd" if get_dist("pillow-simd") is not None else "pillow"
requirements.append(pillow_req + pillow_ver)

Ok, let's try this. What about we go with Pillow>8.3,=8.* - E.g limiting to 8.* but newer than 8.3?

Sounds good!

[ofrimasad]

just remember that many users are installing SG and then installing PyTorch again (because of CUDA compatibility).
So the version of pillow defined in torch will run-over our version.

[BloodAxe]

@ofrimasad here's the problem we have:

1. Pillow of 9.x requires "Colab restart" after installation in order to work properly. I dunno why, but my take is it is imported, then reinstalled but due to way how Notebooks operate you have to apply restart. This action causes a friction of new users trying our tutorials.

2. If we use Pillow of 8.x, "Colab restart" problem goes away. But, we have our tests failed since Pillow 8.x contains know vulnerabilities and our dependency scanners prevent from merging this dependency.

3. Torchvision declare quite wide range of versions: >= 6.2 and != 8.2. In practice in Colab this ends up with 8.4 (I dunno why it does not install the 9.x but I assume because this is the default version in Colab environment).

Solutions:
As of now I see only one more or less "safe" solution is to remove Pillow from our dependency entirely and rely on torchvision's dependency, because it much less strict and there is a chance that Colab environment would have it already and we will not reinstall Pillow => No need to restart kernel.

[BloodAxe]

So what I did is duplicated torchvision requirements for pillow. This makes our dependency bot happy, while also gives much more freedom to re-use installed Pillow (Fixes the problem of restarting kernel in Colab).

[shaydeci]

LGTM