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

Avoid / silence warnings in refactored tests #7905

Merged
merged 2 commits into from
Aug 30, 2023

Conversation

NicolasHug
Copy link
Member

@NicolasHug NicolasHug commented Aug 30, 2023

cc @pmeier

@NicolasHug NicolasHug requested a review from pmeier August 30, 2023 10:33
@pytorch-bot
Copy link

pytorch-bot bot commented Aug 30, 2023

@@ -1375,6 +1375,7 @@ def test_transform_noop(self, make_input, device):
assert_equal(output, input)


@pytest.mark.filterwarnings("ignore:The provided center argument has no effect")
Copy link
Member Author

@NicolasHug NicolasHug Aug 30, 2023

Choose a reason for hiding this comment

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

This silences the following. Maybe we can make the silencing more localized, or just avoid testing unnecessary configurations. Maybe we should assert the warning somewhere. 🤷‍♂️

(pt) ➜  vision git:(njflajnlajn) ✗ pytest test/test_transforms_v2_refactored.py -k rotate
=============================================================== test session starts ===============================================================
platform linux -- Python 3.9.12, pytest-7.0.1, pluggy-1.0.0
rootdir: /home/nicolashug/dev/vision, configfile: pytest.ini
plugins: typeguard-2.13.3, cov-3.0.0, mock-3.7.0, anyio-3.5.0
collected 3625 items / 2978 deselected / 647 selected                                                                                             

test/test_transforms_v2_refactored.py ............................................ssssssssssssssssssssssssssssssssssssssssssss............. [ 15%]
.........................................ssssssssssssssssssssssssssssssssssssssssssssssssssssss.....................ssssss................. [ 37%]
........................................................................................................................................... [ 58%]
........................................................................................................................................... [ 80%]
.................................................................................................................................           [100%]

================================================================ warnings summary =================================================================
test/test_transforms_v2_refactored.py: 96 warnings
  /home/nicolashug/dev/vision/torchvision/transforms/v2/functional/_geometry.py:982: UserWarning: The provided center argument has no effect on the result if expand is True
    warnings.warn("The provided center argument has no effect on the result if expand is True")

test/test_transforms_v2_refactored.py: 96 warnings
  /home/nicolashug/dev/vision/torchvision/transforms/v2/functional/_geometry.py:1021: UserWarning: The provided center argument has no effect on the result if expand is True
    warnings.warn("The provided center argument has no effect on the result if expand is True")

test/test_transforms_v2_refactored.py: 36 warnings
  /home/nicolashug/dev/vision/torchvision/transforms/v2/functional/_geometry.py:1037: UserWarning: The provided center argument has no effect on the result if expand is True
    warnings.warn("The provided center argument has no effect on the result if expand is True")

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we have seen time and time again that in practice center does have an effect with expand=True should we maybe downgrade this into a note in the documentation instead? cc @vfdev-5

Copy link
Collaborator

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Thanks! Should we maybe turn all warnings into errors for this file to prevent this in the future?

@NicolasHug
Copy link
Member Author

Should we maybe turn all warnings into errors for this file to prevent this in the future?

Why not, although

pytestmark = pytest.mark.filterwarnings("error")

from https://docs.pytest.org/en/7.1.x/how-to/capture-warnings.html#pytest-mark-filterwarnings seems to conflict with our filter, so I'll leave it out for now.

@NicolasHug NicolasHug merged commit a2189d6 into pytorch:main Aug 30, 2023
facebook-github-bot pushed a commit that referenced this pull request Sep 6, 2023
Reviewed By: matteobettini

Differential Revision: D48900368

fbshipit-source-id: e8f6166040ab9a190da85a0292c372bc3423200b
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.

3 participants