-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
Fix test_draw_boxes #3631
Fix test_draw_boxes #3631
Conversation
One possible reason can be that a new version of PIL was released 3 days ago. Tests seem to start failing then. This might bring to one question. For As we can see there is extra width param in stable Pillow While 4.1.1 does not have. Have a look here I see that in the It depends on the PIL version installed on your system. Results may vary depending on it. I verified once but I don't think so we have any other plotting dependency. |
You're right, thanks. The failure is caused by the release of PIL 8.2 (tests still pass on master with 8.1). Regarding the PIL dependency: it might indeed be time to bump the minimal version. But more importantly: it seems that we're currently only testing against the latest PIL version, which is a bit risky. Only testing the latest version means that if we're using a PIL API that was only introduced recently, our PIL dependency @fmassa WDYT? |
@@ -120,8 +123,11 @@ def test_draw_boxes(self): | |||
res = Image.fromarray(result.permute(1, 2, 0).contiguous().numpy()) | |||
res.save(path) | |||
|
|||
expected = torch.as_tensor(np.array(Image.open(path))).permute(2, 0, 1) | |||
self.assertTrue(torch.equal(result, expected)) | |||
if PILLOW_VERSION >= (8, 2): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another solution is to set the condition to if PILLOW_VERSION < (8, 2)
and not update the image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
I'm ok adding one extra CI for an older version of PIL as well in a follow-up PR
Summary: * new image * avoid check if pil version is < 8.2 as the reference image would be different Reviewed By: NicolasHug Differential Revision: D27706947 fbshipit-source-id: 4c43289e4ebf742b643112e24a8119fc1f70ac55
Looks like something changed (don't know what TBH) and this test is failing on new PRs like #3628 and #3624
This PR updates the reference image (EDIT: we also avoid the check if PIL_version < 8.2). The new image seems to be more correct as in the current one, the top pixel of the "b" letter seems to be separate from the rest https://github.com/pytorch/vision/blob/master/test/assets/fakedata/draw_boxes_util.png
EDIT: Closes #3637