Port test_datasets_samplers.py to pytest #4037
Conversation
test/test_datasets_samplers.py
Outdated
| def test_random_clip_sampler(self): | ||
| with get_list_of_videos(num_videos=3, sizes=[25, 25, 25]) as video_list: | ||
| video_clips = VideoClips(video_list, 5, 5) | ||
| sampler = RandomClipSampler(video_clips, 3) | ||
| self.assertEqual(len(sampler), 3 * 3) | ||
| assert_equal(len(sampler), 3 * 3) |
There was a problem hiding this comment.
Thanks for the PR @vivekkumar7089 !
Since we're just comparing numbers here (not tensors), we can just rely on a simple form:
| assert_equal(len(sampler), 3 * 3) | |
| assert len(sampler) == 3 * 3 |
Could you please update this one and all of the remaining ones below as well? Thanks!
There was a problem hiding this comment.
OK, I will update.
There was a problem hiding this comment.
@vivekkumar7089 Sorry for the confusion again but that comment above still stands: we should convert assertEqual into assert a == b, not into assert_equal
There was a problem hiding this comment.
No problem, @NicolasHug , I have changed it back to assert a == b.
…test_datasets_samplers-to-pytest
…test_datasets_samplers-to-pytest
test/test_datasets_samplers.py
Outdated
| # remove elements of the first video, to simplify testing | ||
| indices.remove(0) | ||
| indices.remove(1) | ||
| indices = torch.tensor(indices) - 2 | ||
| videos = torch.div(indices, 5, rounding_mode='floor') | ||
| v_idxs, count = torch.unique(videos, return_counts=True) | ||
| assert_equal(v_idxs, torch.tensor([0, 1])) | ||
| assert_equal(count, torch.tensor([3, 3])) | ||
| assert v_idxs == torch.tensor([0, 1]) |
There was a problem hiding this comment.
Sorry for the confusion: when comparing tensors, we should still use assert_equal.
In this file we don't want to change the existing instances of assert_equal.
We should just convert self.assertEqual into assert a == b (and maybe into assert_equal for iterables, but I doubt there are any instances of that)
There was a problem hiding this comment.
@NicolasHug should I need to revert back all those last changes?
There was a problem hiding this comment.
yes please, the existing assert_equal statements (as in master) should be left as-is
…test_datasets_samplers-to-pytest
| def test_random_clip_sampler(self): | ||
| with get_list_of_videos(num_videos=3, sizes=[25, 25, 25]) as video_list: | ||
| video_clips = VideoClips(video_list, 5, 5) | ||
| sampler = RandomClipSampler(video_clips, 3) | ||
| self.assertEqual(len(sampler), 3 * 3) | ||
| assert len(sampler) == 3 * 3 |
test/test_datasets_samplers.py
Outdated
| assert v_idxs == torch.tensor([0, 1, 2]) | ||
| assert count == torch.tensor([3, 3, 3]) |
There was a problem hiding this comment.
Again, here and everywhere else, please do not change the existing assert_equal calls. You should only change the self.assertEqual calls into assert a == b
There was a problem hiding this comment.
Oh, I am sorry, @NicolasHug , for the inconvinience. Actually, I got confused. I will correct them.
There was a problem hiding this comment.
Thanks @vivekkumar7089 :) !
LGTM when green (the current failure in binary_linux_conda_py3.8_cu111 is unrelated)
|
Hey @NicolasHug! You approved or merged this PR, but no labels were added. |
|
Thanks @vivekkumar7089 ! |
Reviewed By: fmassa Differential Revision: D29097735 fbshipit-source-id: efe35d35283981cca6274de580169e12a676e90f
This PR ports
test_datasets_samplers.pytopytestas mentioned in #4033 .