-
Notifications
You must be signed in to change notification settings - Fork 7k
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
test_five_crop,test_ten_crop,test_max_value,test_linear_transformatio… #3975
Conversation
This PR is for #3945 Group G test_five_crop Group D test_autoaugment |
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.
Thanks @harishsdev , I made a quick review.
Could you please move these new tests functions at the end of the file, as asked in #3958 (comment)?
Also there seem to be a new untitled new file with 1999 new lines, we'll need to remove this one :)
test/test_transforms.py
Outdated
for policy in transforms.AutoAugmentPolicy: | ||
for fill in [None, 85, (128, 128, 128)]: |
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.
We'll need to parametrize over policy and fill instead of having nested for loops
test/test_transforms.py
Outdated
for should_vflip in [True, False]: | ||
for single_dim in [True, False]: |
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.
we should parametrize here too
test/test_transforms.py
Outdated
transforms.ToPILImage(), | ||
transforms.CenterCrop((oheight, owidth)), | ||
transforms.ToTensor(), ])(img) | ||
assert_equal(result.sum(), 0, |
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.
as this is just an integer comparison, we should just use assert a == b
as per the instructions in #3945 (comment)
Also I see above that the formatting is a bit odd:
transforms.ToTensor(), ])(img)
Please make sure to preserve the original formatting :)
test/test_transforms.py
Outdated
@pytest.mark.parametrize(should_vflip, [True, False]) | ||
@pytest.mark.parametrize(single_dim, [True, False]) | ||
def test_ten_crop(should_vflip,single_dim): |
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.
This should be:
@pytest.mark.parametrize(should_vflip, [True, False]) | |
@pytest.mark.parametrize(single_dim, [True, False]) | |
def test_ten_crop(should_vflip,single_dim): | |
@pytest.mark.parametrize('should_vflip', [True, False]) | |
@pytest.mark.parametrize('single_dim', [True, False]) | |
def test_ten_crop(should_vflip, single_dim): |
Please make sure to read the instructions at #3945 (comment) and you can take inspiration from https://github.com/pytorch/vision/pull/3907/files to see how to parametrize tests.
Also I really recommend to try your changes locally before you push: it will be much easier for you to see what works and what doesn't work
PR for:-#3945 Group B Group C |
@NicolasHug Thanks you very much for your patience,It gives to motivation while working on this opensource issues,I have raised Single PR for add B,C,D,G Groups,please help to merge them,Thanks for supporting |
@harishsdev thanks for the PR but please consider submitting only 2 groups per PR to keep the review managable, as asked in #3945 Also, the test are failing with: test/test_transforms.py:1777: in <module>
@pytest.mark.parametrize(pt, [(16, 16), (20, 16), (20, 20)])
E NameError: name 'pt' is not defined This is the same issue that I already pointed out in #3975 (comment) Please don't hesitate to ask for specific help, but first I kindly ask that you try running the tests locally before pushing changes, as it will make this process much faster. We have contributing guidelines here: https://github.com/pytorch/vision/blob/master/CONTRIBUTING.md with instructions on how to run the test. |
@NicolasHug Please Help Thanks for your inputs,As per torchvision documentation we have two tests, 1.code formatting 1.for code formatting and typing i use flake8 --config=setup.cfg . This Gives no error 2.Unit testing :- (practice) PS D:\contrib\19283975\exp\vision> pytest test -vvv ========================================================== ERRORS =========================================================== torchvision_init_.py:26 -- Docs: https://docs.pytest.org/en/stable/warnings.html |
You likely have an old version of pytorch. You can update with Also running |
Port test/test_transforms.py to pytest for below transforms
test_five_crop
test_ten_crop
test_max_value
test_linear_transformation
test_autoaugment
test_center_crop
test_center_crop_2
test_color_jitter
PR for :#3945