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

Explicitly copying array in pil_to_tensor #4566

Merged
merged 5 commits into from
Oct 8, 2021

Conversation

jdsgomes
Copy link
Contributor

@jdsgomes jdsgomes commented Oct 7, 2021

In order to avoid UserWarning when calling pil_to_tensor we are making a deep copy of the underlying array when converting from PIL Image to Tensor.

fixes #4508

cc @vfdev-5 @datumbox

@datumbox datumbox changed the title mExplicitly copying array in pil_to_tensor Explicitly copying array in pil_to_tensor Oct 7, 2021
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution Joao.

We can merge after a green CI.

torchvision/transforms/functional.py Show resolved Hide resolved
Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @jdsgomes!

torchvision/transforms/functional.py Outdated Show resolved Hide resolved
@@ -161,6 +161,10 @@ def pil_to_tensor(pic):

Returns:
Tensor: Converted image.

.. note::
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for writing this note, such documentation is important

Let's also add it to to the PILToTensor transform, since it relies on pil_to_tensor.

Also, maybe put it above the Args: section for it to have better visibility?

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Thanks @jdsgomes ! Last comment to fix the docs, but LGTM when addressed

torchvision/transforms/transforms.py Outdated Show resolved Hide resolved
Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
@datumbox datumbox merged commit 85b8fe6 into pytorch:main Oct 8, 2021
datumbox added a commit to datumbox/vision that referenced this pull request Oct 8, 2021
* mExplicitly copying array in pil_to_tensor

* Update torchvision/transforms/functional.py

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

* Adding comments regarding implicit array deep copy in PILToTensor transform

* Update torchvision/transforms/transforms.py

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

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>
datumbox added a commit that referenced this pull request Oct 8, 2021
* mExplicitly copying array in pil_to_tensor

* Update torchvision/transforms/functional.py

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

* Adding comments regarding implicit array deep copy in PILToTensor transform

* Update torchvision/transforms/transforms.py

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

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>

Co-authored-by: Joao Gomes <joaopsgomes@gmail.com>
Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
facebook-github-bot pushed a commit that referenced this pull request Oct 8, 2021
Summary:
* mExplicitly copying array in pil_to_tensor

* Update torchvision/transforms/functional.py

* Adding comments regarding implicit array deep copy in PILToTensor transform

* Update torchvision/transforms/transforms.py

Reviewed By: NicolasHug

Differential Revision: D31505570

fbshipit-source-id: a2ce1c5cf6b70236bc71a79a3d190ddac7d378a4

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>
mszhanyi pushed a commit to mszhanyi/vision that referenced this pull request Oct 19, 2021
* mExplicitly copying array in pil_to_tensor

* Update torchvision/transforms/functional.py

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

* Adding comments regarding implicit array deep copy in PILToTensor transform

* Update torchvision/transforms/transforms.py

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

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>
cyyever pushed a commit to cyyever/vision that referenced this pull request Nov 16, 2021
* mExplicitly copying array in pil_to_tensor

* Update torchvision/transforms/functional.py

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

* Adding comments regarding implicit array deep copy in PILToTensor transform

* Update torchvision/transforms/transforms.py

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

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pil_to_tensor() raises a "NumPy array is not writeable" warning
3 participants