Skip to content

Commit

Permalink
Refactored others type of subotimal asserts (#7672)
Browse files Browse the repository at this point in the history
### Description

As discussed in PR #7609, I tried to break the suboptimal test refactors
to smaller pieces. In this PR all asserts are checking textual content
or instance of a parameter.

Suboptimal Assert: Instead of using statements such as assertIsNone,
assertIsInstance, always simply use assertTrue or assertFalse. This will
decrease the code overall readability and increase the execution time as
extra logic needed.

### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Non-breaking change (fix or new feature that would not break
existing functionality).
- [ ] Breaking change (fix or new feature that would cause existing
functionality to change).
- [ ] New tests added to cover the changes.
- [ ] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [ ] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [ ] In-line docstrings updated.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

---------

Signed-off-by: Han Wang <freddie.wanah@gmail.com>
Signed-off-by: Ben Murray <ben.murray@gmail.com>
Signed-off-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com>
Co-authored-by: Ben Murray <ben.murray@gmail.com>
  • Loading branch information
3 people authored Apr 24, 2024
1 parent c3e4457 commit bfe09b8
Show file tree
Hide file tree
Showing 33 changed files with 111 additions and 112 deletions.
6 changes: 3 additions & 3 deletions tests/test_bundle_ckpt_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ def test_export(self, key_in_ckpt, use_trace):
_, metadata, extra_files = load_net_with_metadata(
ts_file, more_extra_files=["inference.json", "def_args.json"]
)
self.assertTrue("schema" in metadata)
self.assertTrue("meta_file" in json.loads(extra_files["def_args.json"]))
self.assertTrue("network_def" in json.loads(extra_files["inference.json"]))
self.assertIn("schema", metadata)
self.assertIn("meta_file", json.loads(extra_files["def_args.json"]))
self.assertIn("network_def", json.loads(extra_files["inference.json"]))

@parameterized.expand([TEST_CASE_1, TEST_CASE_2, TEST_CASE_3])
def test_default_value(self, key_in_ckpt, use_trace):
Expand Down
17 changes: 9 additions & 8 deletions tests/test_bundle_get_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,25 +51,26 @@ class TestGetBundleData(unittest.TestCase):
def test_get_all_bundles_list(self, params):
with skip_if_downloading_fails():
output = get_all_bundles_list(**params)
self.assertTrue(isinstance(output, list))
self.assertTrue(isinstance(output[0], tuple))
self.assertEqual(len(output[0]), 2)
self.assertIsInstance(output, list)
self.assertIsInstance(output[0], tuple)
self.assertTrue(len(output[0]) == 2)

@parameterized.expand([TEST_CASE_1, TEST_CASE_5])
@skip_if_quick
def test_get_bundle_versions(self, params):
with skip_if_downloading_fails():
output = get_bundle_versions(**params)
self.assertTrue(isinstance(output, dict))
self.assertTrue("latest_version" in output and "all_versions" in output)
self.assertTrue("0.1.0" in output["all_versions"])
self.assertIsInstance(output, dict)
self.assertIn("latest_version", output)
self.assertIn("all_versions", output)
self.assertIn("0.1.0", output["all_versions"])

@parameterized.expand([TEST_CASE_1, TEST_CASE_2])
@skip_if_quick
def test_get_bundle_info(self, params):
with skip_if_downloading_fails():
output = get_bundle_info(**params)
self.assertTrue(isinstance(output, dict))
self.assertIsInstance(output, dict)
for key in ["id", "name", "size", "download_count", "browser_download_url"]:
self.assertTrue(key in output)

Expand All @@ -78,7 +79,7 @@ def test_get_bundle_info(self, params):
def test_get_bundle_info_monaihosting(self, params):
with skip_if_downloading_fails():
output = get_bundle_info(**params)
self.assertTrue(isinstance(output, dict))
self.assertIsInstance(output, dict)
for key in ["name", "browser_download_url"]:
self.assertTrue(key in output)

Expand Down
12 changes: 6 additions & 6 deletions tests/test_bundle_trt_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ def test_trt_export(self, convert_precision, input_shape, dynamic_batch):
_, metadata, extra_files = load_net_with_metadata(
ts_file, more_extra_files=["inference.json", "def_args.json"]
)
self.assertTrue("schema" in metadata)
self.assertTrue("meta_file" in json.loads(extra_files["def_args.json"]))
self.assertTrue("network_def" in json.loads(extra_files["inference.json"]))
self.assertIn("schema", metadata)
self.assertIn("meta_file", json.loads(extra_files["def_args.json"]))
self.assertIn("network_def", json.loads(extra_files["inference.json"]))

@parameterized.expand([TEST_CASE_3, TEST_CASE_4])
@unittest.skipUnless(
Expand Down Expand Up @@ -129,9 +129,9 @@ def test_onnx_trt_export(self, convert_precision, input_shape, dynamic_batch):
_, metadata, extra_files = load_net_with_metadata(
ts_file, more_extra_files=["inference.json", "def_args.json"]
)
self.assertTrue("schema" in metadata)
self.assertTrue("meta_file" in json.loads(extra_files["def_args.json"]))
self.assertTrue("network_def" in json.loads(extra_files["inference.json"]))
self.assertIn("schema", metadata)
self.assertIn("meta_file", json.loads(extra_files["def_args.json"]))
self.assertIn("network_def", json.loads(extra_files["inference.json"]))


if __name__ == "__main__":
Expand Down
6 changes: 3 additions & 3 deletions tests/test_bundle_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,11 @@ def test_train_config(self, config_file):
self.assertListEqual(trainer.check_properties(), [])
# test read / write the properties
dataset = trainer.train_dataset
self.assertTrue(isinstance(dataset, Dataset))
self.assertIsInstance(dataset, Dataset)
inferer = trainer.train_inferer
self.assertTrue(isinstance(inferer, SimpleInferer))
self.assertIsInstance(inferer, SimpleInferer)
# test optional properties get
self.assertTrue(trainer.train_key_metric is None)
self.assertIsNone(trainer.train_key_metric)
trainer.train_dataset = deepcopy(dataset)
trainer.train_inferer = deepcopy(inferer)
# test optional properties set
Expand Down
8 changes: 4 additions & 4 deletions tests/test_component_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,17 @@ def test_add2(self):
self.cs.add("test_obj2", "Test object", test_obj2)

self.assertEqual(len(self.cs), 2)
self.assertTrue("test_obj1" in self.cs)
self.assertTrue("test_obj2" in self.cs)
self.assertIn("test_obj1", self.cs)
self.assertIn("test_obj2", self.cs)

def test_add_def(self):
self.assertFalse("test_func" in self.cs)
self.assertNotIn("test_func", self.cs)

@self.cs.add_def("test_func", "Test function")
def test_func():
return 123

self.assertTrue("test_func" in self.cs)
self.assertIn("test_func", self.cs)

self.assertEqual(len(self.cs), 1)
self.assertEqual(list(self.cs), [("test_func", test_func)])
Expand Down
4 changes: 2 additions & 2 deletions tests/test_compute_ho_ver_maps.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ class ComputeHoVerMapsTests(unittest.TestCase):
def test_horizontal_certical_maps(self, in_type, arguments, mask, hv_mask):
input_image = in_type(mask)
result = ComputeHoVerMaps(**arguments)(input_image)
self.assertTrue(isinstance(result, torch.Tensor))
self.assertTrue(str(result.dtype).split(".")[1] == arguments.get("dtype", "float32"))
self.assertIsInstance(result, torch.Tensor)
self.assertEqual(str(result.dtype).split(".")[1], arguments.get("dtype", "float32"))
assert_allclose(result, hv_mask, type_test="tensor")


Expand Down
4 changes: 2 additions & 2 deletions tests/test_compute_ho_ver_maps_d.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ def test_horizontal_certical_maps(self, in_type, arguments, mask, hv_mask):
for k in mask.keys():
input_image[k] = in_type(mask[k])
result = ComputeHoVerMapsd(keys="mask", **arguments)(input_image)[hv_key]
self.assertTrue(isinstance(result, torch.Tensor))
self.assertTrue(str(result.dtype).split(".")[1] == arguments.get("dtype", "float32"))
self.assertIsInstance(result, torch.Tensor)
self.assertEqual(str(result.dtype).split(".")[1], arguments.get("dtype", "float32"))
assert_allclose(result, hv_mask[hv_key], type_test="tensor")


Expand Down
8 changes: 4 additions & 4 deletions tests/test_concat_itemsd.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def test_tensor_values(self):
"img2": torch.tensor([[0, 1], [1, 2]], device=device),
}
result = ConcatItemsd(keys=["img1", "img2"], name="cat_img")(input_data)
self.assertTrue("cat_img" in result)
self.assertIn("cat_img", result)
result["cat_img"] += 1
assert_allclose(result["img1"], torch.tensor([[0, 1], [1, 2]], device=device))
assert_allclose(result["cat_img"], torch.tensor([[1, 2], [2, 3], [1, 2], [2, 3]], device=device))
Expand All @@ -42,8 +42,8 @@ def test_metatensor_values(self):
"img2": MetaTensor([[0, 1], [1, 2]], device=device),
}
result = ConcatItemsd(keys=["img1", "img2"], name="cat_img")(input_data)
self.assertTrue("cat_img" in result)
self.assertTrue(isinstance(result["cat_img"], MetaTensor))
self.assertIn("cat_img", result)
self.assertIsInstance(result["cat_img"], MetaTensor)
self.assertEqual(result["img1"].meta, result["cat_img"].meta)
result["cat_img"] += 1
assert_allclose(result["img1"], torch.tensor([[0, 1], [1, 2]], device=device))
Expand All @@ -52,7 +52,7 @@ def test_metatensor_values(self):
def test_numpy_values(self):
input_data = {"img1": np.array([[0, 1], [1, 2]]), "img2": np.array([[0, 1], [1, 2]])}
result = ConcatItemsd(keys=["img1", "img2"], name="cat_img")(input_data)
self.assertTrue("cat_img" in result)
self.assertIn("cat_img", result)
result["cat_img"] += 1
np.testing.assert_allclose(result["img1"], np.array([[0, 1], [1, 2]]))
np.testing.assert_allclose(result["cat_img"], np.array([[1, 2], [2, 3], [1, 2], [2, 3]]))
Expand Down
2 changes: 1 addition & 1 deletion tests/test_config_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ def test_function(self, config):
if id in ("compute", "cls_compute"):
parser[f"{id}#_mode_"] = "callable"
func = parser.get_parsed_content(id=id)
self.assertTrue(id in parser.ref_resolver.resolved_content)
self.assertIn(id, parser.ref_resolver.resolved_content)
if id == "error_func":
with self.assertRaises(TypeError):
func(1, 2)
Expand Down
16 changes: 8 additions & 8 deletions tests/test_cucim_dict_transform.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ class TestCuCIMDict(unittest.TestCase):
def test_tramsforms_numpy_single(self, params, input, expected):
input = {"image": input}
output = CuCIMd(keys="image", **params)(input)["image"]
self.assertTrue(output.dtype == expected.dtype)
self.assertTrue(isinstance(output, np.ndarray))
self.assertEqual(output.dtype, expected.dtype)
self.assertIsInstance(output, np.ndarray)
cp.testing.assert_allclose(output, expected)

@parameterized.expand(
Expand All @@ -98,8 +98,8 @@ def test_tramsforms_numpy_batch(self, params, input, expected):
input = {"image": input[cp.newaxis, ...]}
expected = expected[cp.newaxis, ...]
output = CuCIMd(keys="image", **params)(input)["image"]
self.assertTrue(output.dtype == expected.dtype)
self.assertTrue(isinstance(output, np.ndarray))
self.assertEqual(output.dtype, expected.dtype)
self.assertIsInstance(output, np.ndarray)
cp.testing.assert_allclose(output, expected)

@parameterized.expand(
Expand All @@ -116,8 +116,8 @@ def test_tramsforms_cupy_single(self, params, input, expected):
input = {"image": cp.asarray(input)}
expected = cp.asarray(expected)
output = CuCIMd(keys="image", **params)(input)["image"]
self.assertTrue(output.dtype == expected.dtype)
self.assertTrue(isinstance(output, cp.ndarray))
self.assertEqual(output.dtype, expected.dtype)
self.assertIsInstance(output, cp.ndarray)
cp.testing.assert_allclose(output, expected)

@parameterized.expand(
Expand All @@ -134,8 +134,8 @@ def test_tramsforms_cupy_batch(self, params, input, expected):
input = {"image": cp.asarray(input)[cp.newaxis, ...]}
expected = cp.asarray(expected)[cp.newaxis, ...]
output = CuCIMd(keys="image", **params)(input)["image"]
self.assertTrue(output.dtype == expected.dtype)
self.assertTrue(isinstance(output, cp.ndarray))
self.assertEqual(output.dtype, expected.dtype)
self.assertIsInstance(output, cp.ndarray)
cp.testing.assert_allclose(output, expected)


Expand Down
16 changes: 8 additions & 8 deletions tests/test_cucim_transform.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ class TestCuCIM(unittest.TestCase):
)
def test_tramsforms_numpy_single(self, params, input, expected):
output = CuCIM(**params)(input)
self.assertTrue(output.dtype == expected.dtype)
self.assertTrue(isinstance(output, np.ndarray))
self.assertEqual(output.dtype, expected.dtype)
self.assertIsInstance(output, np.ndarray)
cp.testing.assert_allclose(output, expected)

@parameterized.expand(
Expand All @@ -97,8 +97,8 @@ def test_tramsforms_numpy_batch(self, params, input, expected):
input = input[cp.newaxis, ...]
expected = expected[cp.newaxis, ...]
output = CuCIM(**params)(input)
self.assertTrue(output.dtype == expected.dtype)
self.assertTrue(isinstance(output, np.ndarray))
self.assertEqual(output.dtype, expected.dtype)
self.assertIsInstance(output, np.ndarray)
cp.testing.assert_allclose(output, expected)

@parameterized.expand(
Expand All @@ -115,8 +115,8 @@ def test_tramsforms_cupy_single(self, params, input, expected):
input = cp.asarray(input)
expected = cp.asarray(expected)
output = CuCIM(**params)(input)
self.assertTrue(output.dtype == expected.dtype)
self.assertTrue(isinstance(output, cp.ndarray))
self.assertEqual(output.dtype, expected.dtype)
self.assertIsInstance(output, cp.ndarray)
cp.testing.assert_allclose(output, expected)

@parameterized.expand(
Expand All @@ -133,8 +133,8 @@ def test_tramsforms_cupy_batch(self, params, input, expected):
input = cp.asarray(input)[cp.newaxis, ...]
expected = cp.asarray(expected)[cp.newaxis, ...]
output = CuCIM(**params)(input)
self.assertTrue(output.dtype == expected.dtype)
self.assertTrue(isinstance(output, cp.ndarray))
self.assertEqual(output.dtype, expected.dtype)
self.assertIsInstance(output, cp.ndarray)
cp.testing.assert_allclose(output, expected)


Expand Down
2 changes: 1 addition & 1 deletion tests/test_detect_envelope.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def test_value_error(self, arguments, image, method):
elif method == "__call__":
self.assertRaises(ValueError, DetectEnvelope(**arguments), image)
else:
raise ValueError("Expected raising method invalid. Should be __init__ or __call__.")
self.fail("Expected raising method invalid. Should be __init__ or __call__.")


@SkipIfModule("torch.fft")
Expand Down
32 changes: 16 additions & 16 deletions tests/test_ensure_typed.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ def test_array_input(self):
keys="data", data_type=dtype, dtype=np.float32 if dtype == "NUMPY" else None, device="cpu"
)({"data": test_data})["data"]
if dtype == "NUMPY":
self.assertTrue(result.dtype == np.float32)
self.assertTrue(isinstance(result, torch.Tensor if dtype == "tensor" else np.ndarray))
self.assertEqual(result.dtype, np.float32)
self.assertIsInstance(result, torch.Tensor if dtype == "tensor" else np.ndarray)
assert_allclose(result, test_data, type_test=False)
self.assertTupleEqual(result.shape, (2, 2))

Expand All @@ -45,7 +45,7 @@ def test_single_input(self):
for test_data in test_datas:
for dtype in ("tensor", "numpy"):
result = EnsureTyped(keys="data", data_type=dtype)({"data": test_data})["data"]
self.assertTrue(isinstance(result, torch.Tensor if dtype == "tensor" else np.ndarray))
self.assertIsInstance(result, torch.Tensor if dtype == "tensor" else np.ndarray)
if isinstance(test_data, bool):
self.assertFalse(result)
else:
Expand All @@ -56,27 +56,27 @@ def test_string(self):
for dtype in ("tensor", "numpy"):
# string input
result = EnsureTyped(keys="data", data_type=dtype)({"data": "test_string"})["data"]
self.assertTrue(isinstance(result, str))
self.assertIsInstance(result, str)
self.assertEqual(result, "test_string")
# numpy array of string
result = EnsureTyped(keys="data", data_type=dtype)({"data": np.array(["test_string"])})["data"]
self.assertTrue(isinstance(result, np.ndarray))
self.assertIsInstance(result, np.ndarray)
self.assertEqual(result[0], "test_string")

def test_list_tuple(self):
for dtype in ("tensor", "numpy"):
result = EnsureTyped(keys="data", data_type=dtype, wrap_sequence=False, track_meta=True)(
{"data": [[1, 2], [3, 4]]}
)["data"]
self.assertTrue(isinstance(result, list))
self.assertTrue(isinstance(result[0][1], MetaTensor if dtype == "tensor" else np.ndarray))
self.assertIsInstance(result, list)
self.assertIsInstance(result[0][1], MetaTensor if dtype == "tensor" else np.ndarray)
assert_allclose(result[1][0], torch.as_tensor(3), type_test=False)
# tuple of numpy arrays
result = EnsureTyped(keys="data", data_type=dtype, wrap_sequence=False)(
{"data": (np.array([1, 2]), np.array([3, 4]))}
)["data"]
self.assertTrue(isinstance(result, tuple))
self.assertTrue(isinstance(result[0], torch.Tensor if dtype == "tensor" else np.ndarray))
self.assertIsInstance(result, tuple)
self.assertIsInstance(result[0], torch.Tensor if dtype == "tensor" else np.ndarray)
assert_allclose(result[1], torch.as_tensor([3, 4]), type_test=False)

def test_dict(self):
Expand All @@ -92,19 +92,19 @@ def test_dict(self):
)
for key in ("data", "label"):
result = trans[key]
self.assertTrue(isinstance(result, dict))
self.assertTrue(isinstance(result["img"], torch.Tensor if dtype == "tensor" else np.ndarray))
self.assertTrue(isinstance(result["meta"]["size"], torch.Tensor if dtype == "tensor" else np.ndarray))
self.assertIsInstance(result, dict)
self.assertIsInstance(result["img"], torch.Tensor if dtype == "tensor" else np.ndarray)
self.assertIsInstance(result["meta"]["size"], torch.Tensor if dtype == "tensor" else np.ndarray)
self.assertEqual(result["meta"]["path"], "temp/test")
self.assertEqual(result["extra"], None)
assert_allclose(result["img"], torch.as_tensor([1.0, 2.0]), type_test=False)
assert_allclose(result["meta"]["size"], torch.as_tensor([1, 2, 3]), type_test=False)
if dtype == "numpy":
self.assertTrue(trans["data"]["img"].dtype == np.float32)
self.assertTrue(trans["label"]["img"].dtype == np.int8)
self.assertEqual(trans["data"]["img"].dtype, np.float32)
self.assertEqual(trans["label"]["img"].dtype, np.int8)
else:
self.assertTrue(trans["data"]["img"].dtype == torch.float32)
self.assertTrue(trans["label"]["img"].dtype == torch.int8)
self.assertEqual(trans["data"]["img"].dtype, torch.float32)
self.assertEqual(trans["label"]["img"].dtype, torch.int8)


if __name__ == "__main__":
Expand Down
2 changes: 1 addition & 1 deletion tests/test_flipd.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def test_torch(self, spatial_axis, img: torch.Tensor, track_meta: bool, device):
def test_meta_dict(self):
xform = Flipd("image", [0, 1])
res = xform({"image": torch.zeros(1, 3, 4)})
self.assertTrue(res["image"].applied_operations == res["image_transforms"])
self.assertEqual(res["image"].applied_operations, res["image_transforms"])


if __name__ == "__main__":
Expand Down
8 changes: 4 additions & 4 deletions tests/test_freeze_layers.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ def test_freeze_vars(self, device):

for name, param in model.named_parameters():
if "class_layer" in name:
self.assertEqual(param.requires_grad, False)
self.assertFalse(param.requires_grad)
else:
self.assertEqual(param.requires_grad, True)
self.assertTrue(param.requires_grad)

@parameterized.expand(TEST_CASES)
def test_exclude_vars(self, device):
Expand All @@ -53,9 +53,9 @@ def test_exclude_vars(self, device):

for name, param in model.named_parameters():
if "class_layer" in name:
self.assertEqual(param.requires_grad, True)
self.assertTrue(param.requires_grad)
else:
self.assertEqual(param.requires_grad, False)
self.assertFalse(param.requires_grad)


if __name__ == "__main__":
Expand Down
Loading

0 comments on commit bfe09b8

Please sign in to comment.