-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Tests Cleanup #8535
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
base: dev
Are you sure you want to change the base?
Tests Cleanup #8535
Conversation
Signed-off-by: Eric Kerfoot <[email protected]>
Signed-off-by: Eric Kerfoot <[email protected]>
…nt directory when run Signed-off-by: Eric Kerfoot <[email protected]>
Signed-off-by: Eric Kerfoot <[email protected]>
…ghtly Signed-off-by: Eric Kerfoot <[email protected]>
…ently supported PyTorch versions Signed-off-by: Eric Kerfoot <[email protected]>
Signed-off-by: Eric Kerfoot <[email protected]>
Signed-off-by: Eric Kerfoot <[email protected]>
WalkthroughThis change removes all version-dependent test skips (e.g., Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
for more information, see https://pre-commit.ci
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.
Actionable comments posted: 10
🔭 Outside diff range comments (6)
tests/integration/test_auto3dseg_hpo.py (1)
121-130
: Broaden download guard to include bundle generationNetwork/remote access can occur during
generate()
. Keep it inside the skip-on-download-fail context to avoid flaky failures.ConfigParser.export_config_file(data_src, data_src_cfg) -with skip_if_downloading_fails(): - bundle_generator = BundleGen( - algo_path=work_dir, - data_stats_filename=da_output_yaml, - data_src_cfg_name=data_src_cfg, - templates_path_or_url=get_testing_algo_template_path(), - ) -bundle_generator.generate(work_dir, num_fold=1) +with skip_if_downloading_fails(): + bundle_generator = BundleGen( + algo_path=work_dir, + data_stats_filename=da_output_yaml, + data_src_cfg_name=data_src_cfg, + templates_path_or_url=get_testing_algo_template_path(), + ) + bundle_generator.generate(work_dir, num_fold=1)tests/bundle/test_bundle_download.py (1)
171-179
: Bug: hash check is outside the loop and never runsThe
if file == "network.json":
executes after the loop;file
is the last item, so the hash check is skipped.- for file in bundle_files: - file_path = os.path.join(tempdir, bundle_name, file) - self.assertTrue(os.path.exists(file_path)) - if file == "network.json": - self.assertTrue(check_hash(filepath=file_path, val=hash_val)) + for file in bundle_files: + file_path = os.path.join(tempdir, bundle_name, file) + self.assertTrue(os.path.exists(file_path)) + if file == "network.json": + self.assertTrue(check_hash(filepath=file_path, val=hash_val))tests/apps/detection/networks/test_retinanet_detector.py (1)
129-133
: Replace tautological asserts with real checks.
assert len(result) == len(result)
is a no-op. Assert against input size instead.- result = detector.forward(input_data) - assert len(result) == len(result) + result = detector.forward(input_data) + assert len(result) == input_data.shape[0] input_data = [torch.randn(input_shape[1:]) for _ in range(random.randint(1, 9))] - result = detector.forward(input_data) - assert len(result) == len(result) + result = detector.forward(input_data) + assert len(result) == len(input_data)And similarly in the naive-detector test:
- result = detector.forward(input_data) - assert len(result) == len(result) + result = detector.forward(input_data) + assert len(result) == input_data.shape[0] input_data = [torch.randn(input_shape[1:]) for _ in range(random.randint(1, 9))] - result = detector.forward(input_data) - assert len(result) == len(result) + result = detector.forward(input_data) + assert len(result) == len(input_data)Also applies to: 163-167
tests/integration/test_auto3dseg_ensemble.py (1)
147-156
: Extend download-skip guard to cover bundle generationCurrently only the constructor is guarded; failures during generate() won’t be skipped. Wrap generate() and get_history() in the context.
- with skip_if_downloading_fails(): - bundle_generator = BundleGen( - algo_path=self.work_dir, - data_stats_filename=self.da_output_yaml, - data_src_cfg_name=self.data_src_cfg_name, - templates_path_or_url=get_testing_algo_template_path(), - ) - bundle_generator.generate(self.work_dir, num_fold=1) - history = bundle_generator.get_history() + with skip_if_downloading_fails(): + bundle_generator = BundleGen( + algo_path=self.work_dir, + data_stats_filename=self.da_output_yaml, + data_src_cfg_name=self.data_src_cfg_name, + templates_path_or_url=get_testing_algo_template_path(), + ) + bundle_generator.generate(self.work_dir, num_fold=1) + history = bundle_generator.get_history()tests/networks/blocks/test_selfattention.py (2)
211-223
: Bug: loop body not applied to both causal casesOnly the last loop iteration’s input_param is used. Move the body into the loop so both causal=True/False are tested.
- @skipUnless(has_einops, "Requires einops") - def test_flash_attention(self): - for causal in [True, False]: - input_param = {"hidden_size": 360, "num_heads": 4, "input_size": (16, 32), "causal": causal} - device = "cuda:0" if torch.cuda.is_available() else "cpu" - block_w_flash_attention = SABlock(**input_param, use_flash_attention=True).to(device) - block_wo_flash_attention = SABlock(**input_param, use_flash_attention=False).to(device) - block_wo_flash_attention.load_state_dict(block_w_flash_attention.state_dict()) - test_data = torch.randn(2, 512, 360).to(device) - - out_1 = block_w_flash_attention(test_data) - out_2 = block_wo_flash_attention(test_data) - assert_allclose(out_1, out_2, atol=1e-4) + @skipUnless(has_einops, "Requires einops") + def test_flash_attention(self): + device = "cuda:0" if torch.cuda.is_available() else "cpu" + for causal in [True, False]: + input_param = {"hidden_size": 360, "num_heads": 4, "input_size": (16, 32), "causal": causal} + block_w_flash_attention = SABlock(**input_param, use_flash_attention=True).to(device) + block_wo_flash_attention = SABlock(**input_param, use_flash_attention=False).to(device) + block_wo_flash_attention.load_state_dict(block_w_flash_attention.state_dict()) + test_data = torch.randn(2, 512, 360).to(device) + + out_1 = block_w_flash_attention(test_data) + out_2 = block_wo_flash_attention(test_data) + assert_allclose(out_1, out_2, atol=1e-4)
225-236
: Bug: undefined variableuse_combined_linear
NameError will occur. Set explicit value or parameterize it; simplest is to fix to False here since the test focuses on include_fc.
input_param = { "hidden_size": 360, "num_heads": 4, "dropout_rate": 0.0, "rel_pos_embedding": None, "input_size": (16, 32), "include_fc": include_fc, - "use_combined_linear": use_combined_linear, + "use_combined_linear": False, }
🧹 Nitpick comments (23)
tests/networks/nets/test_vit.py (3)
101-110
: Redundant JIT scripting; consider relying on test_script_save only.
torch.jit.script(net)
is functionally duplicated bytest_script_save(...)
. Dropping the direct script call reduces runtime without losing coverage.def test_script(self, input_param, input_shape, _): net = ViT(**(input_param)) net.eval() - with torch.no_grad(): - torch.jit.script(net) test_data = torch.randn(input_shape) test_script_save(net, test_data)
118-129
: Fix typos: “acess” → “access” for clarity.Rename locals and comments for readability.
- # no data in the matrix - no_matrix_acess_blk = ViT(in_channels=in_channels, img_size=img_size, patch_size=patch_size) - no_matrix_acess_blk(torch.randn(in_shape)) - assert isinstance(no_matrix_acess_blk.blocks[0].attn.att_mat, torch.Tensor) + # no data in the matrix + no_matrix_access_blk = ViT(in_channels=in_channels, img_size=img_size, patch_size=patch_size) + no_matrix_access_blk(torch.randn(in_shape)) + assert isinstance(no_matrix_access_blk.blocks[0].attn.att_mat, torch.Tensor) # no of elements is zero - assert no_matrix_acess_blk.blocks[0].attn.att_mat.nelement() == 0 + assert no_matrix_access_blk.blocks[0].attn.att_mat.nelement() == 0 - # be able to acess the attention matrix - matrix_acess_blk = ViT(in_channels=in_channels, img_size=img_size, patch_size=patch_size, save_attn=True) - matrix_acess_blk(torch.randn(in_shape)) - assert matrix_acess_blk.blocks[0].attn.att_mat.shape == (in_shape[0], 12, 216, 216) + # be able to access the attention matrix + matrix_access_blk = ViT(in_channels=in_channels, img_size=img_size, patch_size=patch_size, save_attn=True) + matrix_access_blk(torch.randn(in_shape)) + assert matrix_access_blk.blocks[0].attn.att_mat.shape == (in_shape[0], 12, 216, 216)
121-123
: Prefer unittest assertions over bare asserts for consistency.Use
self.assert*
to align with unittest style and better failure diagnostics.- assert isinstance(no_matrix_access_blk.blocks[0].attn.att_mat, torch.Tensor) + self.assertIsInstance(no_matrix_access_blk.blocks[0].attn.att_mat, torch.Tensor) - assert no_matrix_access_blk.blocks[0].attn.att_mat.nelement() == 0 + self.assertEqual(no_matrix_access_blk.blocks[0].attn.att_mat.nelement(), 0) - assert matrix_access_blk.blocks[0].attn.att_mat.shape == (in_shape[0], 12, 216, 216) + self.assertEqual(matrix_access_blk.blocks[0].attn.att_mat.shape, (in_shape[0], 12, 216, 216))Also applies to: 128-128
tests/transforms/test_detect_envelope.py (1)
55-55
: Nit: inline comment mismatches the provided argument.Arg sets
axis=2
but comment says “axis argument to 1”.- {"axis": 2}, # set axis argument to 1 + {"axis": 2}, # set axis argument to 2tests/data/meta_tensor/test_meta_tensor.py (2)
37-41
: Simplify DTYPES/TESTS construction (remove one-element containers).Current DTYPES elements are wrapped in one-element lists to enable unpacking. Flattening simplifies intent and avoids subtle bugs if
TEST_DEVICES
ever stops using 1-tuples.Apply:
- DTYPES = [[torch.float32], [torch.float64], [torch.float16], [torch.int64], [torch.int32], [None]] + DTYPES = [torch.float32, torch.float64, torch.float16, torch.int64, torch.int32, None] # Replace nested loops with dict_product - TESTS = [(*params["device"], *params["dtype"]) for params in dict_product(device=TEST_DEVICES, dtype=DTYPES)] + TESTS = [(*params["device"], params["dtype"]) for params in dict_product(device=TEST_DEVICES, dtype=DTYPES)]
178-180
: Remove redundant clone or assert both.
a = m.clone(memory_format=torch.preserve_format)
is immediately overwritten. Either drop it, or add an assertion to cover thememory_format
path.Minimal cleanup:
- a = m.clone(memory_format=torch.preserve_format) - a = m.clone() + a = m.clone()tests/integration/test_auto3dseg_hpo.py (1)
48-52
: Fix docstring: it mentions CUDA but guards OptunaDocstring is misleading.
def skip_if_no_optuna(obj): - """ - Skip the unit tests if torch.cuda.is_available is False. - """ + """ + Skip tests if Optuna is unavailable. + """ return unittest.skipUnless(has_optuna, "Skipping optuna tests")(obj)tests/bundle/test_bundle_download.py (4)
115-137
: Make expected tensor dtype explicitBe robust to global default dtype changes by setting dtype explicitly.
TESTCASE_NGC_WEIGHTS = { "key": "model.0.conv.unit0.adn.N.bias", - "value": torch.tensor( + "value": torch.tensor( [ -0.0705, -0.0937, -0.0422, -0.2068, 0.1023, -0.2007, -0.0883, 0.0018, -0.1719, 0.0116, 0.0285, -0.0044, 0.1223, -0.1287, -0.1858, 0.0460, - ] - ), + ], + dtype=torch.float32, + ), }
462-486
: NGC download/load test: add explicit device for clarityNot required, but being explicit avoids surprises if defaults change.
- model = load( - name=bundle_name, source="ngc", version=version, bundle_dir=tempdir, remove_prefix=remove_prefix - ) + model = load( + name=bundle_name, + source="ngc", + version=version, + bundle_dir=tempdir, + remove_prefix=remove_prefix, + device="cpu", + )
450-451
: Remove stray print in testsAvoid noisy stdout in test runs.
- print(file_path)
66-67
: Unused test case constant
TEST_CASE_6_HF
is defined but not used. Remove or add a test using it.tests/metrics/test_cumulative_average_dist.py (1)
35-35
: Grammar nit in comment“It’s” -> “its”.
- avg_meter = CumulativeAverage() # each process rank has it's own AverageMeter + avg_meter = CumulativeAverage() # each process rank has its own AverageMetertests/apps/detection/networks/test_retinanet_detector.py (1)
28-28
: Remove unused variable.
num_anchors
(Line 28) is never used. Drop it.-num_anchors = 7
tests/networks/test_convert_to_onnx.py (1)
23-23
: Minor consistency: import optional_import from monai.utils.Other files import optional_import from monai.utils; align here for consistency.
-from tests.test_utils import SkipIfNoModule, optional_import, skip_if_quick +from tests.test_utils import SkipIfNoModule, skip_if_quick +from monai.utils import optional_importtests/networks/blocks/test_crossattention.py (2)
49-49
: Rename test class for clarityThis class tests CrossAttentionBlock but is named TestResBlock. Consider renaming to avoid confusion.
-class TestResBlock(unittest.TestCase): +class TestCrossAttentionBlock(unittest.TestCase):
146-159
: Fix typos: “acess” -> “access” and minor variable namesTidy up typos in comments and variable names for readability.
- # be not able to access the matrix - no_matrix_acess_blk = CrossAttentionBlock( + # be not able to access the matrix + no_matrix_access_blk = CrossAttentionBlock( hidden_size=hidden_size, num_heads=num_heads, dropout_rate=dropout_rate ) - no_matrix_acess_blk(torch.randn(input_shape)) - assert isinstance(no_matrix_acess_blk.att_mat, torch.Tensor) + no_matrix_access_blk(torch.randn(input_shape)) + assert isinstance(no_matrix_access_blk.att_mat, torch.Tensor) # no of elements is zero - assert no_matrix_acess_blk.att_mat.nelement() == 0 + assert no_matrix_access_blk.att_mat.nelement() == 0 @@ - # be able to acess the attention matrix. - matrix_acess_blk = CrossAttentionBlock( + # be able to access the attention matrix. + matrix_access_blk = CrossAttentionBlock( hidden_size=hidden_size, num_heads=num_heads, dropout_rate=dropout_rate, save_attn=True ) - matrix_acess_blk(torch.randn(input_shape)) - assert matrix_acess_blk.att_mat.shape == (input_shape[0], input_shape[0], input_shape[1], input_shape[1]) + matrix_access_blk(torch.randn(input_shape)) + assert matrix_access_blk.att_mat.shape == (input_shape[0], input_shape[0], input_shape[1], input_shape[1])tests/networks/blocks/test_selfattention.py (2)
54-54
: Rename test class for clarityThis class tests SABlock; name it accordingly.
-class TestResBlock(unittest.TestCase): +class TestSABlock(unittest.TestCase):
146-159
: Fix typos: “acess” -> “access” and variable namesClean up typos to improve readability.
- # be not able to access the matrix - no_matrix_acess_blk = SABlock(hidden_size=hidden_size, num_heads=num_heads, dropout_rate=dropout_rate) - no_matrix_acess_blk(torch.randn(input_shape)) - assert isinstance(no_matrix_acess_blk.att_mat, torch.Tensor) + # be not able to access the matrix + no_matrix_access_blk = SABlock(hidden_size=hidden_size, num_heads=num_heads, dropout_rate=dropout_rate) + no_matrix_access_blk(torch.randn(input_shape)) + assert isinstance(no_matrix_access_blk.att_mat, torch.Tensor) # no of elements is zero - assert no_matrix_acess_blk.att_mat.nelement() == 0 + assert no_matrix_access_blk.att_mat.nelement() == 0 @@ - # be able to acess the attention matrix - matrix_acess_blk = SABlock( + # be able to access the attention matrix + matrix_access_blk = SABlock( hidden_size=hidden_size, num_heads=num_heads, dropout_rate=dropout_rate, save_attn=True ) - matrix_acess_blk(torch.randn(input_shape)) - assert matrix_acess_blk.att_mat.shape == (input_shape[0], input_shape[0], input_shape[1], input_shape[1]) + matrix_access_blk(torch.randn(input_shape)) + assert matrix_access_blk.att_mat.shape == (input_shape[0], input_shape[0], input_shape[1], input_shape[1])tests/networks/nets/test_autoencoderkl.py (1)
28-29
: Remove unused tqdm import
tqdm, has_tqdm
aren’t used. Trim to keep tests lean.-tqdm, has_tqdm = optional_import("tqdm", name="tqdm") -_, has_einops = optional_import("einops") +_, has_einops = optional_import("einops")tests/inferers/test_zarr_avg_merger.py (1)
348-356
: Prefer addCleanup for robust teardownRegister cleanups in setUp so they run even if setUp partially fails; drop explicit tearDown.
- def setUp(self): - self.orig_settings = np.seterr(divide="ignore", invalid="ignore") - self.temp_dir = TemporaryDirectory() - self.merged_name = os.path.join(self.temp_dir.name, "merged.zarr") - - def tearDown(self): - np.seterr(**self.orig_settings) - self.temp_dir.cleanup() + def setUp(self): + # suppress numpy warnings during tests and restore automatically + self._orig_seterr = np.seterr(divide="ignore", invalid="ignore") + self.addCleanup(np.seterr, **self._orig_seterr) + # temp workspace for zarr stores + self.temp_dir = TemporaryDirectory() + self.addCleanup(self.temp_dir.cleanup) + self.merged_name = os.path.join(self.temp_dir.name, "merged.zarr")tests/networks/layers/test_gaussian_filter.py (3)
44-84
: Set expected arrays dtype to float32Avoid dtype-driven tolerance surprises across platforms by fixing expected arrays to float32.
-EXPECTED_1D = np.array( +EXPECTED_1D = np.array( [0.5654129, 0.68915915, 0.79146194, 0.8631974, 0.8998163, 0.8998163, 0.8631973, 0.79146194, 0.6891592, 0.5654129] -) + , dtype=np.float32) -EXPECTED_2D = np.array( +EXPECTED_2D = np.array( [[0.13239081, 0.13932934, 0.13239081], [0.13932936, 0.14663152, 0.13932936], [0.13239081, 0.13932934, 0.13239081]] -) + , dtype=np.float32) -EXPECTED_3D = np.array( +EXPECTED_3D = np.array( [ ... ] -) + , dtype=np.float32) -EXPECTED_3D_SIGMAS = np.array( +EXPECTED_3D_SIGMAS = np.array( [ ... ] -) + , dtype=np.float32)
95-99
: Use assertRaises over empty-pattern assertRaisesRegexEmpty regex adds no value and can mask message regressions.
- with self.assertRaisesRegex(ValueError, ""): + with self.assertRaises(ValueError): GaussianFilter(3, [3, 2], 3)
104-105
: Prefer torch.testing.assert_close; avoid NumPy roundtripsKeeps tensors on device/dtype, reduces conversion overhead.
- np.testing.assert_allclose(g(a).cpu().numpy(), expected, rtol=1e-5) + actual = g(a) + expected_t = torch.as_tensor(expected, dtype=actual.dtype, device=actual.device) + torch.testing.assert_close(actual, expected_t, rtol=1e-5)- np.testing.assert_allclose(g(a).cpu().numpy(), unsqueeze_left(expected, len(oargs)), rtol=rtol) + actual = g(a) + expected_t = torch.as_tensor(unsqueeze_left(expected, len(oargs)), dtype=actual.dtype, device=actual.device) + torch.testing.assert_close(actual, expected_t, rtol=rtol)Also applies to: 112-112
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (64)
tests/__init__.py
(0 hunks)tests/apps/detection/networks/test_retinanet.py
(1 hunks)tests/apps/detection/networks/test_retinanet_detector.py
(1 hunks)tests/apps/detection/utils/test_anchor_box.py
(1 hunks)tests/apps/maisi/networks/test_autoencoderkl_maisi.py
(0 hunks)tests/apps/maisi/networks/test_controlnet_maisi.py
(0 hunks)tests/apps/test_auto3dseg_bundlegen.py
(1 hunks)tests/apps/vista3d/test_point_based_window_inferer.py
(1 hunks)tests/bundle/test_bundle_download.py
(3 hunks)tests/data/meta_tensor/test_meta_tensor.py
(1 hunks)tests/data/test_shuffle_buffer.py
(0 hunks)tests/fl/monai_algo/test_fl_monai_algo_dist.py
(1 hunks)tests/handlers/test_handler_tb_image.py
(0 hunks)tests/inferers/test_zarr_avg_merger.py
(5 hunks)tests/integration/test_auto3dseg_ensemble.py
(1 hunks)tests/integration/test_auto3dseg_hpo.py
(1 hunks)tests/integration/test_integration_autorunner.py
(1 hunks)tests/integration/test_integration_gpu_customization.py
(1 hunks)tests/integration/test_integration_lazy_samples.py
(1 hunks)tests/integration/test_integration_nnunet_bundle.py
(1 hunks)tests/integration/test_integration_nnunetv2_runner.py
(1 hunks)tests/integration/test_integration_workers.py
(1 hunks)tests/integration/test_reg_loss_integration.py
(0 hunks)tests/losses/image_dissimilarity/test_global_mutual_information_loss.py
(1 hunks)tests/losses/test_ds_loss.py
(1 hunks)tests/losses/test_perceptual_loss.py
(1 hunks)tests/metrics/test_cumulative_average_dist.py
(1 hunks)tests/networks/blocks/test_CABlock.py
(1 hunks)tests/networks/blocks/test_crossattention.py
(1 hunks)tests/networks/blocks/test_fpn_block.py
(1 hunks)tests/networks/blocks/test_patchembedding.py
(1 hunks)tests/networks/blocks/test_selfattention.py
(1 hunks)tests/networks/blocks/test_subpixel_upsample.py
(1 hunks)tests/networks/blocks/warp/test_warp.py
(1 hunks)tests/networks/layers/test_gaussian_filter.py
(3 hunks)tests/networks/layers/test_hilbert_transform.py
(0 hunks)tests/networks/nets/test_autoencoderkl.py
(1 hunks)tests/networks/nets/test_dints_network.py
(1 hunks)tests/networks/nets/test_segresnet_ds.py
(1 hunks)tests/networks/nets/test_unetr.py
(1 hunks)tests/networks/nets/test_vista3d.py
(1 hunks)tests/networks/nets/test_vit.py
(1 hunks)tests/networks/nets/test_vqvae.py
(0 hunks)tests/networks/test_bundle_onnx_export.py
(1 hunks)tests/networks/test_convert_to_onnx.py
(1 hunks)tests/networks/test_varnet.py
(1 hunks)tests/ngc_bundle_download.py
(0 hunks)tests/test_set_visible_devices.py
(0 hunks)tests/transforms/spatial/test_grid_patch.py
(1 hunks)tests/transforms/spatial/test_grid_patchd.py
(1 hunks)tests/transforms/spatial/test_rand_grid_patch.py
(1 hunks)tests/transforms/spatial/test_rand_grid_patchd.py
(1 hunks)tests/transforms/test_detect_envelope.py
(1 hunks)tests/transforms/test_fourier.py
(0 hunks)tests/transforms/test_resample_backends.py
(1 hunks)tests/transforms/test_resize.py
(1 hunks)tests/transforms/test_resized.py
(1 hunks)tests/transforms/test_signal_fillempty.py
(0 hunks)tests/transforms/test_signal_fillemptyd.py
(0 hunks)tests/transforms/test_signal_rand_add_squarepulse.py
(0 hunks)tests/transforms/test_signal_rand_add_squarepulse_partial.py
(0 hunks)tests/utils/test_pad_mode.py
(1 hunks)tests/utils/test_set_determinism.py
(1 hunks)tests/visualize/test_plot_2d_or_3d_image.py
(1 hunks)
💤 Files with no reviewable changes (15)
- tests/apps/maisi/networks/test_controlnet_maisi.py
- tests/transforms/test_signal_fillempty.py
- tests/data/test_shuffle_buffer.py
- tests/transforms/test_signal_fillemptyd.py
- tests/integration/test_reg_loss_integration.py
- tests/transforms/test_signal_rand_add_squarepulse.py
- tests/handlers/test_handler_tb_image.py
- tests/transforms/test_signal_rand_add_squarepulse_partial.py
- tests/networks/nets/test_vqvae.py
- tests/transforms/test_fourier.py
- tests/apps/maisi/networks/test_autoencoderkl_maisi.py
- tests/test_set_visible_devices.py
- tests/init.py
- tests/ngc_bundle_download.py
- tests/networks/layers/test_hilbert_transform.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit Configuration File
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
tests/apps/detection/utils/test_anchor_box.py
tests/networks/test_varnet.py
tests/networks/test_convert_to_onnx.py
tests/metrics/test_cumulative_average_dist.py
tests/transforms/test_detect_envelope.py
tests/apps/detection/networks/test_retinanet_detector.py
tests/networks/test_bundle_onnx_export.py
tests/networks/nets/test_vit.py
tests/utils/test_pad_mode.py
tests/fl/monai_algo/test_fl_monai_algo_dist.py
tests/losses/test_perceptual_loss.py
tests/apps/detection/networks/test_retinanet.py
tests/data/meta_tensor/test_meta_tensor.py
tests/apps/vista3d/test_point_based_window_inferer.py
tests/integration/test_integration_nnunet_bundle.py
tests/integration/test_integration_gpu_customization.py
tests/networks/nets/test_vista3d.py
tests/integration/test_integration_workers.py
tests/apps/test_auto3dseg_bundlegen.py
tests/transforms/spatial/test_grid_patchd.py
tests/integration/test_integration_lazy_samples.py
tests/transforms/spatial/test_grid_patch.py
tests/integration/test_auto3dseg_hpo.py
tests/networks/blocks/warp/test_warp.py
tests/networks/nets/test_unetr.py
tests/transforms/test_resample_backends.py
tests/networks/blocks/test_CABlock.py
tests/networks/blocks/test_subpixel_upsample.py
tests/utils/test_set_determinism.py
tests/integration/test_integration_nnunetv2_runner.py
tests/networks/nets/test_dints_network.py
tests/visualize/test_plot_2d_or_3d_image.py
tests/transforms/spatial/test_rand_grid_patchd.py
tests/networks/blocks/test_crossattention.py
tests/networks/blocks/test_fpn_block.py
tests/integration/test_integration_autorunner.py
tests/losses/image_dissimilarity/test_global_mutual_information_loss.py
tests/transforms/spatial/test_rand_grid_patch.py
tests/transforms/test_resized.py
tests/losses/test_ds_loss.py
tests/networks/blocks/test_patchembedding.py
tests/networks/nets/test_segresnet_ds.py
tests/bundle/test_bundle_download.py
tests/transforms/test_resize.py
tests/networks/layers/test_gaussian_filter.py
tests/integration/test_auto3dseg_ensemble.py
tests/networks/nets/test_autoencoderkl.py
tests/inferers/test_zarr_avg_merger.py
tests/networks/blocks/test_selfattention.py
🧬 Code Graph Analysis (45)
tests/apps/detection/utils/test_anchor_box.py (1)
tests/test_utils.py (2)
assert_allclose
(100-140)test_script_save
(739-758)
tests/networks/test_varnet.py (1)
tests/test_utils.py (1)
test_script_save
(739-758)
tests/metrics/test_cumulative_average_dist.py (1)
tests/test_utils.py (2)
DistCall
(412-561)DistTestCase
(398-409)
tests/transforms/test_detect_envelope.py (1)
tests/test_utils.py (1)
assert_allclose
(100-140)
tests/apps/detection/networks/test_retinanet_detector.py (1)
tests/test_utils.py (2)
skip_if_quick
(204-211)test_script_save
(739-758)
tests/networks/test_bundle_onnx_export.py (1)
tests/test_utils.py (3)
SkipIfNoModule
(214-223)command_line_tests
(825-835)skip_if_windows
(252-256)
tests/networks/nets/test_vit.py (1)
tests/test_utils.py (3)
dict_product
(868-885)skip_if_quick
(204-211)test_script_save
(739-758)
tests/utils/test_pad_mode.py (1)
tests/test_utils.py (1)
dict_product
(868-885)
tests/fl/monai_algo/test_fl_monai_algo_dist.py (1)
tests/test_utils.py (4)
DistCall
(412-561)DistTestCase
(398-409)SkipIfNoModule
(214-223)skip_if_no_cuda
(245-249)
tests/losses/test_perceptual_loss.py (1)
tests/test_utils.py (3)
assert_allclose
(100-140)skip_if_downloading_fails
(144-169)skip_if_quick
(204-211)
tests/apps/detection/networks/test_retinanet.py (1)
tests/test_utils.py (4)
dict_product
(868-885)skip_if_quick
(204-211)test_onnx_save
(761-782)test_script_save
(739-758)
tests/data/meta_tensor/test_meta_tensor.py (1)
tests/test_utils.py (3)
assert_allclose
(100-140)dict_product
(868-885)skip_if_no_cuda
(245-249)
tests/apps/vista3d/test_point_based_window_inferer.py (1)
tests/test_utils.py (1)
skip_if_quick
(204-211)
tests/integration/test_integration_nnunet_bundle.py (1)
tests/test_utils.py (3)
skip_if_downloading_fails
(144-169)skip_if_no_cuda
(245-249)skip_if_quick
(204-211)
tests/integration/test_integration_gpu_customization.py (1)
tests/test_utils.py (4)
get_testing_algo_template_path
(77-84)skip_if_downloading_fails
(144-169)skip_if_no_cuda
(245-249)skip_if_quick
(204-211)
tests/networks/nets/test_vista3d.py (1)
tests/test_utils.py (1)
skip_if_quick
(204-211)
tests/integration/test_integration_workers.py (1)
tests/test_utils.py (4)
DistTestCase
(398-409)TimedCall
(564-654)skip_if_no_cuda
(245-249)skip_if_quick
(204-211)
tests/apps/test_auto3dseg_bundlegen.py (1)
tests/test_utils.py (4)
get_testing_algo_template_path
(77-84)skip_if_downloading_fails
(144-169)skip_if_no_cuda
(245-249)skip_if_quick
(204-211)
tests/transforms/spatial/test_grid_patchd.py (1)
tests/test_utils.py (1)
assert_allclose
(100-140)
tests/integration/test_integration_lazy_samples.py (1)
tests/test_utils.py (2)
DistTestCase
(398-409)skip_if_quick
(204-211)
tests/integration/test_auto3dseg_hpo.py (1)
tests/test_utils.py (3)
get_testing_algo_template_path
(77-84)skip_if_downloading_fails
(144-169)skip_if_no_cuda
(245-249)
tests/networks/blocks/warp/test_warp.py (1)
tests/test_utils.py (4)
SkipIfNoModule
(214-223)download_url_or_skip_test
(785-788)skip_if_quick
(204-211)testing_data_config
(67-74)
tests/networks/nets/test_unetr.py (1)
tests/test_utils.py (3)
dict_product
(868-885)skip_if_quick
(204-211)test_script_save
(739-758)
tests/transforms/test_resample_backends.py (1)
tests/test_utils.py (2)
assert_allclose
(100-140)is_tf32_env
(181-201)
tests/networks/blocks/test_CABlock.py (1)
tests/test_utils.py (2)
assert_allclose
(100-140)dict_product
(868-885)
tests/networks/blocks/test_subpixel_upsample.py (1)
tests/test_utils.py (1)
test_script_save
(739-758)
tests/utils/test_set_determinism.py (1)
tests/test_utils.py (1)
skip_if_no_cuda
(245-249)
tests/integration/test_integration_nnunetv2_runner.py (1)
tests/test_utils.py (3)
skip_if_downloading_fails
(144-169)skip_if_no_cuda
(245-249)skip_if_quick
(204-211)
tests/networks/nets/test_dints_network.py (1)
tests/test_utils.py (2)
skip_if_quick
(204-211)test_script_save
(739-758)
tests/visualize/test_plot_2d_or_3d_image.py (1)
tests/test_utils.py (1)
SkipIfNoModule
(214-223)
tests/transforms/spatial/test_rand_grid_patchd.py (1)
tests/test_utils.py (1)
assert_allclose
(100-140)
tests/networks/blocks/test_crossattention.py (1)
tests/test_utils.py (2)
assert_allclose
(100-140)dict_product
(868-885)
tests/networks/blocks/test_fpn_block.py (1)
tests/test_utils.py (1)
test_script_save
(739-758)
tests/integration/test_integration_autorunner.py (1)
tests/test_utils.py (4)
get_testing_algo_template_path
(77-84)skip_if_downloading_fails
(144-169)skip_if_no_cuda
(245-249)skip_if_quick
(204-211)
tests/losses/image_dissimilarity/test_global_mutual_information_loss.py (1)
tests/test_utils.py (3)
download_url_or_skip_test
(785-788)skip_if_quick
(204-211)testing_data_config
(67-74)
tests/transforms/spatial/test_rand_grid_patch.py (1)
tests/test_utils.py (1)
assert_allclose
(100-140)
tests/transforms/test_resized.py (1)
tests/test_utils.py (3)
NumpyImageTestCase2D
(683-696)assert_allclose
(100-140)test_local_inversion
(810-822)
tests/losses/test_ds_loss.py (1)
tests/test_utils.py (1)
test_script_save
(739-758)
tests/networks/blocks/test_patchembedding.py (1)
tests/test_utils.py (1)
dict_product
(868-885)
tests/networks/nets/test_segresnet_ds.py (1)
tests/test_utils.py (2)
dict_product
(868-885)test_script_save
(739-758)
tests/transforms/test_resize.py (1)
tests/test_utils.py (3)
NumpyImageTestCase2D
(683-696)assert_allclose
(100-140)is_tf32_env
(181-201)
tests/networks/layers/test_gaussian_filter.py (3)
monai/utils/misc.py (1)
unsqueeze_left
(913-915)tests/test_utils.py (1)
test_is_quick
(177-178)monai/networks/layers/simplelayers.py (1)
GaussianFilter
(542-595)
tests/integration/test_auto3dseg_ensemble.py (1)
tests/test_utils.py (4)
get_testing_algo_template_path
(77-84)skip_if_downloading_fails
(144-169)skip_if_no_cuda
(245-249)skip_if_quick
(204-211)
tests/networks/nets/test_autoencoderkl.py (1)
tests/test_utils.py (2)
skip_if_downloading_fails
(144-169)testing_data_config
(67-74)
tests/networks/blocks/test_selfattention.py (1)
tests/test_utils.py (2)
assert_allclose
(100-140)test_script_save
(739-758)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: packaging
- GitHub Check: flake8-py3 (mypy)
- GitHub Check: build-docs
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-os (macOS-latest)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-pytorch (2.6.0)
🔇 Additional comments (50)
tests/fl/monai_algo/test_fl_monai_algo_dist.py (1)
26-26
: All legacy version-skip decorators and imports have been removed
tests/fl/monai_algo/test_fl_monai_algo_dist.py
no longer imports or usesSkipIfBeforePyTorchVersion
/SkipIfAtLeastPyTorchVersion
.- RG output only shows class definitions in
tests/test_utils.py
, with no remaining usages.torch.fft
calls are correctly gated viaoptional_import
in transforms and tests.Approved to merge.
tests/networks/nets/test_vit.py (1)
21-21
: Confirm minimum PyTorch version requirementNo explicit PyTorch version pin was found in pyproject.toml, setup.cfg, requirements*/environment*.yml or CI workflows. Please verify that the project’s declared minimum PyTorch version meets or exceeds the version for which you removed SkipIfBeforePyTorchVersion.
tests/data/meta_tensor/test_meta_tensor.py (1)
35-35
: Import cleanup aligns with PR goals; LGTM.Dropping version-gated utilities and consolidating imports is consistent with the test cleanup.
TEST_DEVICES
+dict_product
keep parametrization tidy;skip_if_no_cuda
still guards CUDA-only tests.tests/integration/test_auto3dseg_hpo.py (1)
28-28
: Import cleanup LGTMRemoving version-gated skip is consistent with PR goals; no residual usage here.
tests/bundle/test_bundle_download.py (2)
36-37
: Add skip_if_windows import: OKWindows skip is appropriate for the new NGC tests.
98-113
: NGC test cases: sensible and pinnedCases are clear; pinning the version ensures determinism.
tests/utils/test_pad_mode.py (1)
21-21
: Import cleanup LGTMRemoval of version skip is consistent;
dict_product
usage is appropriate.tests/metrics/test_cumulative_average_dist.py (1)
21-21
: Import cleanup LGTMRemoval of version-based skip matches PR scope.
tests/apps/detection/utils/test_anchor_box.py (1)
21-22
: Import cleanup LGTMConsistent with removing legacy version checks.
tests/losses/image_dissimilarity/test_global_mutual_information_loss.py (1)
23-23
: Cleanup confirmed: version-guard import removed.Matches the direction to simplify tests by dropping old PyTorch checks.
tests/transforms/spatial/test_grid_patch.py (1)
22-22
: Import cleanup looks good.Removing the unused version-skip helpers is consistent with the PR’s goals.
tests/transforms/spatial/test_grid_patchd.py (1)
21-21
: Import cleanup looks good.Consistent with removing legacy version-based skips.
tests/transforms/test_resized.py (1)
25-25
: Removal of version-based skip helper import is appropriate.Ensures tests run uniformly on supported versions.
tests/networks/test_bundle_onnx_export.py (1)
23-23
: Import cleanup (version skip) — OKRemoved SkipIfBeforePyTorchVersion import; class decorators adjusted accordingly. Matches stated cleanup.
tests/apps/vista3d/test_point_based_window_inferer.py (1)
23-23
: Import cleanup — OKRemoved version skip import; class relies on skip_if_quick only. Consistent with test cleanup goals.
tests/networks/nets/test_dints_network.py (1)
22-22
: Import cleanup — OKDropped version-based decorator import; tests now run on supported baselines. Looks good.
tests/losses/test_perceptual_loss.py (1)
21-21
: Import cleanup — OKRemoved version-based skip import; torchvision gating via skipUnless remains. All good.
tests/networks/blocks/warp/test_warp.py (1)
25-25
: Import cleanup — OKRemoved version-based skip import as intended. No functional impact on tests.
tests/integration/test_integration_workers.py (1)
21-21
: No remaining legacy skips or FFT guards detected
A full scan showsSkipIfBeforePyTorchVersion
andSkipIfAtLeastPyTorchVersion
only in their class definitions undertests/test_utils.py
(no uses elsewhere) and notorch.fft
availability checks. All legacy version-based skips and FFT guards have been removed.tests/networks/test_varnet.py (1)
23-23
: Removed version-skip import is correct.TorchScript test now runs under supported versions;
test_script_save
uses CPU and is stable for CI.tests/apps/detection/networks/test_retinanet.py (1)
23-23
: Import pruning matches policy to remove version guards.Class-level skips still appropriately gate on
torchvision
and quick mode.tests/networks/blocks/test_patchembedding.py (1)
24-24
: Good: dropped unused version-dependent import.No functional impact; test gating via
einops
remains intact.tests/integration/test_integration_lazy_samples.py (1)
29-29
: Consistent cleanup of version-based imports.Still gated by
skip_if_quick
; no change in behavior for supported envs.tests/transforms/test_resample_backends.py (1)
25-25
: Version-gated skip removed — OKImport cleanup aligns with dropping legacy version checks. Tests now run unconditionally on supported PyTorch versions. No further changes needed.
tests/networks/nets/test_unetr.py (1)
21-21
: Import cleanup matches removal of version checksGood to drop
SkipIfBeforePyTorchVersion
.test_script
remains focused ontorch.jit.script
with CPU-only save viatest_script_save
. Looks solid.tests/utils/test_set_determinism.py (1)
21-21
: Removed version-based skip import — OKCleanup is appropriate given raised minimum PyTorch version. CUDA-guarded tests remain correctly decorated.
tests/transforms/test_resize.py (1)
25-25
: RemovedSkipIfAtLeastPyTorchVersion
import — OKMatches the intent to eliminate version-based skips. Resize tests should now run across all supported versions.
tests/visualize/test_plot_2d_or_3d_image.py (1)
23-23
: Import cleanup aligns with PR goals.Removing version-based skip import is correct; remaining decorators cover optional deps.
tests/integration/test_integration_nnunet_bundle.py (1)
31-31
: Dropped version‐check import safely — no residual usage found.
Search acrosstests/**/*.py
only returns the class definition intests/test_utils.py:266
and no active uses elsewhere. Dropping the import is safe.tests/integration/test_integration_nnunetv2_runner.py (1)
25-25
: Consistent removal of version-based skip.Matches the cleanup strategy; env-based skips remain intact.
tests/networks/nets/test_segresnet_ds.py (1)
21-21
: Import cleanup — good.Obsolete version guard removed; utilities import remains correct.
tests/transforms/spatial/test_rand_grid_patchd.py (1)
22-22
: All legacy PyTorch version guards removedVerified no references to SkipIfBeforePyTorchVersion or SkipIfAtLeastPyTorchVersion in tests/, and no legacy FFT‐availability gates remain. Safe to merge.
tests/networks/nets/test_vista3d.py (1)
22-22
: Import cleanup: OKDropping the version-based skip import is consistent with supported versions; class remains guarded by quick-test skip.
tests/integration/test_integration_gpu_customization.py (1)
27-27
: Simplified imports: looks goodRemoval of
SkipIfBeforePyTorchVersion
matches the PR goals; remaining decorators sufficiently gate the test.tests/networks/blocks/test_subpixel_upsample.py (1)
23-23
: Decorator removal: approvedTorchScript test now runs unconditionally on supported versions;
test_script_save
already forces CPU, so CI stability is unchanged.tests/apps/test_auto3dseg_bundlegen.py (1)
29-29
: Import pruning: approvedConsistent with eliminating legacy version gates; existing CUDA/quick decorators provide proper test gating.
tests/integration/test_auto3dseg_ensemble.py (1)
35-35
: Removed legacy version-gated import — consistent with supported versionsImport cleanup aligns with the PR objective. No functional impact here.
tests/networks/blocks/test_CABlock.py (1)
24-24
: Removed version-gated import — good cleanupDecorator removal is appropriate given raised minimum PyTorch support. Tests still gated on einops presence, which is sufficient.
tests/networks/blocks/test_fpn_block.py (1)
25-25
: Dropped version gate import — LGTMTorchScript helper remains; version checks are unnecessary with updated support matrix.
tests/transforms/spatial/test_rand_grid_patch.py (1)
24-24
: Removed version-gate import — consistent with cleanupNo dependency on old torch APIs here; safe change.
tests/losses/test_ds_loss.py (1)
22-22
: Removed version-gate import — OKTorchScript test uses CPU via test_script_save; drop of version gating is fine with updated minimum.
tests/networks/blocks/test_crossattention.py (2)
25-25
: Import cleanup LGTMRemoving version-based test utilities is consistent with the PR scope. Nothing else in this file references the old decorators.
25-25
: No remaining version-skips and torch ≥2.4.1 covers flash-attention in tests
rg
only found theSkipIfBeforePyTorchVersion
class definition intests/test_utils.py
(no usages).setup.cfg
andrequirements.txt
both pintorch>=2.4.1
, which exceeds flash-attention’s PyTorch requirements.All clear—no further changes needed.
tests/networks/blocks/test_selfattention.py (1)
25-25
: Import cleanup LGTMRemoval of version-based decorators and keeping only needed utilities looks good.
tests/networks/nets/test_autoencoderkl.py (2)
26-26
: Import cleanup LGTMVersion-based skip removal is consistent; no remaining references in this file.
31-31
: No action required—minimum PyTorch (>=2.4.1) supports torch.load(weights_only=True)
- requirements.txt, setup.cfg and pyproject.toml all pin torch>=2.4.1
- weights_only was introduced in PyTorch 1.10
- no remaining
SkipIfBeforePyTorchVersion
guards against weights_only usagetests/inferers/test_zarr_avg_merger.py (4)
14-16
: Imports align with new test isolationUsing os and TemporaryDirectory matches the new setup/teardown flow. No issues.
156-159
: Defer directory store creation via placeholder — goodThis keeps all artifacts in the temp dir and avoids cwd side effects.
443-449
: Explicit store in error-path tests — goodKeeps artifacts in the temp dir and consistent with the new setup.
452-453
: unittest main guard is fineBenign for pytest discovery; enables standalone execution.
Signed-off-by: Eric Kerfoot <[email protected]>
import unittest | ||
from tempfile import TemporaryDirectory |
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.
I like the use of the tempfile
module here. There is at least one test file that relies on creating temp files at tests/testing_data/temp
, the tests at tests/apps/pathology/test_lesion_froc.py
.
I understand many of the files in tests/testing_data
are testing assets that can be reused, but it would be nice to implement this tempfile
module behaviour to all temporary files.
/build |
Fixes #8534.
Description
This cleans up the tests further as discussed in the issue. This also additionally updates the zarr tests to save the .zarr directories to a temp directory rather than the current one, this is just cleaner when running local tests.
Types of changes
./runtests.sh -f -u --net --coverage
../runtests.sh --quick --unittests --disttests
.make html
command in thedocs/
folder.