Update functional.py

[DonghweeYoon]

Correct the documentation
i is the y_coordinate, j is the x_coordinate

[codecov-io]

Codecov Report

Merging #1323 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1323      +/-   ##
==========================================
- Coverage   65.85%   65.82%   -0.04%     
==========================================
  Files          75       75              
  Lines        5782     5782              
  Branches      884      884              
==========================================
- Hits         3808     3806       -2     
  Misses       1710     1710              
- Partials      264      266       +2

Impacted Files	Coverage Δ
torchvision/transforms/functional.py	71.09% <ø> (-0.29%)	⬇️
torchvision/transforms/transforms.py	80.94% <0%> (-0.2%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a91fe72...cf5a1d2. Read the comment docs.

[pmeier]

Actually, the current comment is correct. I'll try to explain:

The variable i describes the vertical or y direction and j the horizontal or x direction. You can deduce this from the fact that i is added to the height h, which would make no sense if i would represent a horizontal coordinate.

vision/torchvision/transforms/functional.py

Line 368 in a91fe72

return img.crop((j, i, j + w, i + h))

The same is of course true for j.

From the pillow documentation for Image.crop()

box – The crop rectangle, as a (left, upper, right, lower)-tuple

Thus, our call is indeed correct since j is the left part and i the upper part in

vision/torchvision/transforms/functional.py

Lines 357 to 358 in a91fe72

	i (int): i in (i,j) i.e coordinates of the upper left corner.
	j (int): j in (i,j) i.e coordinates of the upper left corner.

---

However, I agree that the current documentation does not reflect this well. We should update it to avoid any misunderstandings in the future.

[DonghweeYoon]

Thank you for the answer. I thought the documentation of the method is ambiguous. I hope it will be clarified soon. 2019년 9월 11일 (수) 오후 4:46, Philip Meier <[email protected]>님이 작성:

…

Actually, the current comment is correct. I'll try to explain: The variable i describes the vertical or *y* direction and j the horizontal or *x* direction. You can deduce this from the fact that i is added to the height h, which would make no sense if i would represent a horizontal coordinate. https://github.com/pytorch/vision/blob/a91fe7221b55c55dbbc23c23aecf33d470a5c08e/torchvision/transforms/functional.py#L368 The same is of course true for j. From the pillow documentation for Image.crop() <https://pillow.readthedocs.io/en/stable/reference/Image.html#PIL.Image.Image.crop> box – The crop rectangle, as a (left, upper, right, lower)-tuple Thus, our call is indeed correct since j is the *left* part and i the *upper* part in https://github.com/pytorch/vision/blob/a91fe7221b55c55dbbc23c23aecf33d470a5c08e/torchvision/transforms/functional.py#L357-L358 ------------------------------ However, I agree that the current documentation does not reflect this well. We should update it to avoid any misunderstandings in the future. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1323?email_source=notifications&email_token=AGWBBT5Y3QLUPHTIXFMX6CTQJCO6JA5CNFSM4IVN7XDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6NS6YQ#issuecomment-530263906>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGWBBT4IOMOPFO5KAZBOUVTQJCO6JANCNFSM4IVN7XDA> .

[fmassa]

One of the problems here is that Pillow uses a different convention than PyTorch for representing the sizes, (width, height instead of height, width), and this can bring some confusion.

We decided to keep the PyTorch convention, where we represent it as height, width, and thus the indices are passed as y, x (in plane coordinates), which are also represented as i, j.

If those indexing conventions are a bit confusing, check the numpy.meshgrid documentation for the indexing argument, which explains this in a bit more detail.

Can you update the PR to make the description a bit more clear?

[fmassa]

Superseeded by #1388