syncing with upstream a bit #65
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces a systematic update across multiple test files in the Datumaro project. The primary changes involve modifying import statements and dataset path definitions. Specifically, the changes replace relative imports with absolute imports for Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
tests/integration/cli/test_info.py (1)
Line range hint
16-21: Consider using get_test_asset_path for test assets.The current path construction using string manipulation on
__file__could be fragile. According to the PR objectives, we should be utilizingget_test_asset_pathfor consistent test asset access.Consider replacing the current path construction with:
-DUMMY_DATASET_DIR = osp.join( - __file__[: __file__.rfind(osp.join("tests", ""))], - "tests", - "assets", - "coco_dataset", - "coco_instances", -) +from tests.utils.test_utils import get_test_asset_path + +DUMMY_DATASET_DIR = get_test_asset_path('coco_dataset/coco_instances')tests/integration/cli/test_kitti_raw_format.py (1)
Line range hint
12-18: Standardize test asset path handlingThe current path construction using string slicing is brittle and inconsistent with other files in the PR that use
get_test_asset_path.Consider replacing with:
-DUMMY_DATASET_DIR = osp.join( - __file__[: __file__.rfind(osp.join("tests", ""))], - "tests", - "assets", - "kitti_dataset", - "kitti_raw", -) +DUMMY_DATASET_DIR = get_test_asset_path("kitti_dataset", "kitti_raw")
🧹 Nitpick comments (7)
tests/unit/test_fractal_image_generator.py (1)
Line range hint
15-15: Address TODO comment about updating links.The TODO comment about updating links should be addressed, especially since it's related to the test's xfail status.
Would you like me to help track this TODO by creating a GitHub issue?
tests/integration/cli/test_generator.py (1)
Line range hint
13-13: Consider tracking the TODO comment in an issue.The TODO comment about model file download issues should be tracked for future resolution.
Would you like me to create a GitHub issue to track this TODO item?
tests/unit/test_kinetics_format.py (1)
6-16: Consider reorganizing imports for better readability.The imports should be organized in standard groups:
- Standard library imports
- Third-party imports
- Local imports
Consider reorganizing like this:
import os.path as osp from unittest import TestCase +from datumaro.components.annotation import Label +from datumaro.components.dataset import Dataset, DatasetItem +from datumaro.components.environment import Environment from datumaro.components.media import Video +from datumaro.plugins.kinetics_format import KineticsImporter from tests.requirements import Requirements, mark_requirement from tests.utils.assets import get_test_asset_path from tests.utils.test_utils import compare_datasets DUMMY_DATASET_DIR = get_test_asset_path("kinetics_dataset") - -from datumaro.components.annotation import Label -from datumaro.components.dataset import Dataset, DatasetItem -from datumaro.components.environment import Environment -from datumaro.plugins.kinetics_format import KineticsImportertests/integration/cli/test_sly_point_cloud_format.py (1)
Line range hint
12-14: Consider using get_test_asset_path for DUMMY_DATASET_DIRFor consistency with other test files, consider updating the dataset path construction to use
get_test_asset_path.-DUMMY_DATASET_DIR = osp.join( - __file__[: __file__.rfind(osp.join("tests", ""))], "tests", "assets", "sly_pointcloud_dataset" -) +DUMMY_DATASET_DIR = get_test_asset_path("sly_pointcloud_dataset")tests/unit/test_align_celeba_format.py (1)
24-26: Simplify multi-line path constructionThe multi-line formatting for a simple two-argument function call adds unnecessary complexity.
Consider simplifying to:
-DUMMY_ALIGN_DATASET_DIR_WITH_META_FILE = get_test_asset_path( - "align_celeba_dataset", "dataset_with_meta_file" -) +DUMMY_ALIGN_DATASET_DIR_WITH_META_FILE = get_test_asset_path("align_celeba_dataset", "dataset_with_meta_file")tests/integration/cli/test_yolo_format.py (1)
12-12: LGTM! Consider extending path improvements.The change to absolute import is good. However, I notice that this file still uses manual path construction for test assets (e.g., in
test_can_save_and_load_yolo_dataset). Consider usingget_test_asset_pathhere as well for consistency.Example refactor for the yolo_dir path:
- yolo_dir = osp.join( - __file__[: __file__.rfind(osp.join("tests", ""))], - "tests", - "assets", - *self.ASSET_PATH, - ) + yolo_dir = get_test_asset_path(*self.ASSET_PATH)tests/unit/test_imagenet_txt_format.py (1)
13-14: LGTM! Consider moving constants to the top.The updates to import statements and dataset paths maintain consistency with other test files. However, consider moving the
DUMMY_DATASET_*constants to the top of the file, before the test classes, as they are used within the test methods. This would improve readability and follow Python's convention of defining constants at the module level.import os.path as osp from unittest import TestCase import numpy as np + +DUMMY_DATASET_DIR = get_test_asset_path("imagenet_txt_dataset", "basic") +DUMMY_DATASET_WITH_CUSTOM_LABELS_DIR = get_test_asset_path("imagenet_txt_dataset", "custom_labels") +DUMMY_DATASET_WITH_NO_LABELS_DIR = get_test_asset_path("imagenet_txt_dataset", "no_labels") from datumaro.components.annotation import AnnotationType, Label, LabelCategoriesAlso applies to: 170-172
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (80)
tests/integration/cli/test_compare.py(1 hunks)tests/integration/cli/test_describe_downloads.py(1 hunks)tests/integration/cli/test_detect_format.py(1 hunks)tests/integration/cli/test_download.py(1 hunks)tests/integration/cli/test_filter.py(1 hunks)tests/integration/cli/test_generator.py(1 hunks)tests/integration/cli/test_image_zip_format.py(1 hunks)tests/integration/cli/test_info.py(1 hunks)tests/integration/cli/test_kitti_raw_format.py(1 hunks)tests/integration/cli/test_merge.py(1 hunks)tests/integration/cli/test_patch.py(1 hunks)tests/integration/cli/test_project.py(1 hunks)tests/integration/cli/test_revpath.py(1 hunks)tests/integration/cli/test_sly_point_cloud_format.py(1 hunks)tests/integration/cli/test_transform.py(1 hunks)tests/integration/cli/test_utils.py(1 hunks)tests/integration/cli/test_video.py(1 hunks)tests/integration/cli/test_voc_format.py(1 hunks)tests/integration/cli/test_yolo_format.py(1 hunks)tests/unit/algorithms/test_RISE.py(1 hunks)tests/unit/data_formats/datumaro/test_datumaro_format.py(3 hunks)tests/unit/data_formats/test_common_semantic_segmentation_format.py(1 hunks)tests/unit/data_formats/test_mapillary_vistas_format.py(1 hunks)tests/unit/data_formats/test_synthia_format.py(1 hunks)tests/unit/test_ade20k2017_format.py(1 hunks)tests/unit/test_ade20k2020_format.py(1 hunks)tests/unit/test_align_celeba_format.py(1 hunks)tests/unit/test_annotation_util.py(1 hunks)tests/unit/test_api.py(1 hunks)tests/unit/test_brats_format.py(1 hunks)tests/unit/test_brats_numpy_format.py(1 hunks)tests/unit/test_camvid_format.py(2 hunks)tests/unit/test_celeba_format.py(1 hunks)tests/unit/test_cifar_format.py(2 hunks)tests/unit/test_cityscapes_format.py(1 hunks)tests/unit/test_coco_format.py(1 hunks)tests/unit/test_common_super_resolution_format.py(1 hunks)tests/unit/test_compare.py(1 hunks)tests/unit/test_config.py(1 hunks)tests/unit/test_cvat_format.py(1 hunks)tests/unit/test_dataset.py(1 hunks)tests/unit/test_fractal_image_generator.py(2 hunks)tests/unit/test_icdar_format.py(1 hunks)tests/unit/test_image.py(1 hunks)tests/unit/test_image_dir_format.py(1 hunks)tests/unit/test_image_zip_format.py(2 hunks)tests/unit/test_imagenet_format.py(2 hunks)tests/unit/test_imagenet_txt_format.py(2 hunks)tests/unit/test_images.py(1 hunks)tests/unit/test_kinetics_format.py(1 hunks)tests/unit/test_kitti_format.py(1 hunks)tests/unit/test_kitti_raw_format.py(1 hunks)tests/unit/test_labelme_format.py(2 hunks)tests/unit/test_lfw_format.py(2 hunks)tests/unit/test_market1501_format.py(2 hunks)tests/unit/test_masks.py(1 hunks)tests/unit/test_mnist_csv_format.py(2 hunks)tests/unit/test_mnist_format.py(2 hunks)tests/unit/test_mot_format.py(2 hunks)tests/unit/test_mots_format.py(1 hunks)tests/unit/test_mpii_format.py(1 hunks)tests/unit/test_mpii_json_format.py(1 hunks)tests/unit/test_ndr.py(1 hunks)tests/unit/test_nyu_depth_v2_format.py(1 hunks)tests/unit/test_ops.py(2 hunks)tests/unit/test_project.py(3 hunks)tests/unit/test_sampler.py(2 hunks)tests/unit/test_sly_pointcloud_format.py(1 hunks)tests/unit/test_splitter.py(1 hunks)tests/unit/test_telemetry_utils.py(1 hunks)tests/unit/test_tfrecord_format.py(2 hunks)tests/unit/test_transforms.py(1 hunks)tests/unit/test_util.py(1 hunks)tests/unit/test_validator.py(1 hunks)tests/unit/test_vgg_face2_format.py(2 hunks)tests/unit/test_video.py(1 hunks)tests/unit/test_voc_format.py(2 hunks)tests/unit/test_vott_csv_format.py(1 hunks)tests/unit/test_vott_json_format.py(1 hunks)tests/unit/test_widerface_format.py(2 hunks)
✅ Files skipped from review due to trivial changes (25)
- tests/unit/test_util.py
- tests/integration/cli/test_image_zip_format.py
- tests/unit/test_sly_pointcloud_format.py
- tests/integration/cli/test_project.py
- tests/integration/cli/test_describe_downloads.py
- tests/integration/cli/test_transform.py
- tests/integration/cli/test_merge.py
- tests/integration/cli/test_voc_format.py
- tests/unit/test_compare.py
- tests/unit/test_splitter.py
- tests/unit/test_masks.py
- tests/unit/test_video.py
- tests/unit/test_ndr.py
- tests/unit/test_image.py
- tests/unit/test_images.py
- tests/unit/test_telemetry_utils.py
- tests/integration/cli/test_compare.py
- tests/unit/algorithms/test_RISE.py
- tests/integration/cli/test_filter.py
- tests/integration/cli/test_revpath.py
- tests/unit/test_tfrecord_format.py
- tests/unit/test_sampler.py
- tests/unit/test_validator.py
- tests/unit/test_config.py
- tests/unit/test_dataset.py
👮 Files not reviewed due to content moderation or server errors (6)
- tests/unit/test_kitti_raw_format.py
- tests/unit/test_cvat_format.py
- tests/unit/test_ops.py
- tests/unit/test_project.py
- tests/unit/test_voc_format.py
- tests/unit/test_coco_format.py
🔇 Additional comments (71)
tests/unit/data_formats/test_synthia_format.py (2)
13-14: LGTM! Import changes improve clarity and maintainability.
The switch to absolute imports and introduction of get_test_asset_path makes the code more explicit and maintainable.
17-25: LGTM! Test asset paths are now consistently managed.
The use of get_test_asset_path standardizes test asset path handling and improves maintainability.
Let's verify that all referenced test assets exist:
✅ Verification successful
All test asset paths exist and are correctly structured
All referenced Synthia dataset paths exist in the expected locations under ./tests/assets/synthia_dataset/:
labels_segm_datasetcolor_segm_datasetdataset_with_custom_labelmapdataset_with_meta_file
Note: While dataset_with_meta_file exists in multiple test asset directories, this is expected as it's a common test scenario across different dataset formats.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all Synthia test assets exist in the expected locations
# Check if the base synthia_dataset directory exists
fd -t d "synthia_dataset$" -x echo "Found base directory: {}"
# Check if all required subdirectories exist
for subdir in "labels_segm_dataset" "color_segm_dataset" "dataset_with_custom_labelmap" "dataset_with_meta_file"; do
fd -t d "$subdir$" -x echo "Found test directory: {}"
done
Length of output: 1716
tests/unit/test_api.py (1)
5-5: LGTM! Import path change improves clarity.
The switch from relative to absolute import enhances maintainability and follows Python best practices for test organization.
tests/integration/cli/test_utils.py (1)
8-9: LGTM! Import organization is clean and consistent.
The absolute imports improve clarity and maintainability. The pylint disable for unused-import is appropriate as it's a test dependency.
tests/integration/cli/test_info.py (1)
13-13: LGTM! Import change is consistent with the codebase.
The switch to absolute import aligns with the changes across other test files.
tests/unit/test_nyu_depth_v2_format.py (1)
13-14: LGTM! Verify test asset path existence.
The changes align well with the PR objectives:
- Moving to absolute imports improves clarity
- Using
get_test_asset_pathcentralizes asset management
Let's verify the test asset path exists:
Also applies to: 17-17
✅ Verification successful
Test asset path exists and is correctly referenced
The nyu_depth_v2_dataset directory is present at the expected location tests/assets/nyu_depth_v2_dataset/, confirming that the asset path changes are valid and the test dependencies are properly maintained.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the nyu_depth_v2_dataset exists in test assets
fd "nyu_depth_v2_dataset" --type d
Length of output: 69
tests/unit/test_fractal_image_generator.py (1)
11-11: LGTM! Verify synthetic dataset path existence.
The changes align with the PR objectives, improving asset path management consistency.
Let's verify the synthetic dataset path exists:
Also applies to: 36-36
✅ Verification successful
Path verification successful - synthetic dataset exists and contains test images
The synthetic dataset path tests/assets/synthetic_dataset/images exists and contains the required test images (000000.png, 000001.png, 000002.png) that are used in the test case.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the synthetic_dataset/images directory exists
fd "synthetic_dataset/images" --type d
Length of output: 38
Script:
#!/bin/bash
# Let's check the content of the test file to understand the path usage
cat tests/unit/test_fractal_image_generator.py
Length of output: 1785
Script:
#!/bin/bash
# Let's check the test_utils.py to understand get_test_asset_path implementation
cat tests/utils/test_utils.py
Length of output: 13366
Script:
#!/bin/bash
# Let's check if there's a helper function for getting test assets path
rg "get_test_asset_path" -A 2
Length of output: 27308
Script:
#!/bin/bash
# Let's check if the test assets directory exists and contains synthetic_dataset/images
ls -la tests/assets/synthetic_dataset/images 2>/dev/null || echo "Directory not found"
Length of output: 337
tests/integration/cli/test_video.py (1)
5-6: LGTM! Verify make_sample_video import path.
The import path changes improve consistency and align with the PR objectives.
Let's verify the new import path exists:
✅ Verification successful
Import path for make_sample_video is correct and verified
The make_sample_video function exists in tests/unit/test_video.py and is properly defined. The import statement in tests/integration/cli/test_video.py correctly references this function from its location.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify make_sample_video exists in the new location
ast-grep --pattern 'def make_sample_video($$$)' 'tests/unit/test_video.py'
Length of output: 74
Script:
#!/bin/bash
# Let's first check if the file exists
ls -l tests/unit/test_video.py
# If file exists, let's see its content to verify the function
cat tests/unit/test_video.py
# As a fallback, let's search for any test_video.py files in the repository
fd test_video.py
Length of output: 7978
tests/unit/test_common_super_resolution_format.py (2)
13-14: LGTM! Import changes align with project standardization.
The switch to absolute imports from the tests package improves code clarity and maintainability.
17-17: LGTM! Path management improvement.
Using get_test_asset_path centralizes test asset management and aligns with the PR's objective of consistent asset access across tests.
tests/integration/cli/test_generator.py (1)
9-9: LGTM! Import standardization.
The change to absolute imports aligns with the project-wide standardization effort.
tests/unit/test_kinetics_format.py (1)
9-9: LGTM! Consistent asset path management.
Using get_test_asset_path aligns with the PR's objective of standardizing test asset access.
tests/unit/test_brats_format.py (1)
13-14: LGTM! Import and path management improvements
The changes align with the PR objectives by:
- Standardizing imports using absolute paths
- Utilizing the
get_test_asset_pathutility for consistent asset management
Also applies to: 17-17
tests/unit/test_image_dir_format.py (1)
10-10: LGTM! Standardized import path
The change from relative to absolute import aligns with the project-wide standardization effort.
tests/unit/test_brats_numpy_format.py (1)
13-14: LGTM! Consistent import and path management changes
The changes follow the project-wide standardization pattern.
Let's verify the existence of both dataset paths:
Also applies to: 17-17
✅ Verification successful
Both dataset paths exist as expected
The verification confirms that both required dataset paths are present in the test assets:
tests/assets/brats_dataset/tests/assets/brats_numpy_dataset/
This validates that the path changes in the imports and DUMMY_DATASET_DIR are correct and the necessary test assets are available.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of both brats dataset paths
# Expected: Both paths should exist as they are used in tests
echo "Checking for brats dataset paths..."
fd -t d "brats.*dataset$"
Length of output: 170
tests/unit/test_annotation_util.py (1)
11-11: LGTM: Import path standardization
The change from relative to absolute import aligns with the PR's objective of improving test organization and maintainability.
tests/unit/test_vott_csv_format.py (2)
13-14: LGTM: Import path standardization
The change from relative to absolute imports and addition of get_test_asset_path import aligns with the PR's objective.
17-18: LGTM: Consistent test asset path handling
Using get_test_asset_path for dataset directories improves consistency in test asset management across the test suite.
tests/integration/cli/test_download.py (1)
8-8: LGTM: Import path standardization
The change from relative to absolute import is consistent with the PR's standardization effort across test files.
tests/unit/test_ade20k2017_format.py (1)
17-17: LGTM! Path handling improvements
The changes correctly implement the standardized asset path handling using get_test_asset_path, improving maintainability and consistency across test files.
Also applies to: 20-20, 22-22
tests/integration/cli/test_sly_point_cloud_format.py (1)
9-9: LGTM! Import statement standardization
The change from relative to absolute import aligns with the project's standardization efforts.
tests/unit/test_vott_json_format.py (1)
13-14: LGTM! Consistent path handling implementation
The changes correctly implement:
- Standardized imports
- Consistent asset path handling using
get_test_asset_path
Also applies to: 17-20
tests/unit/test_image_zip_format.py (2)
11-12: LGTM! Import changes align with project standardization
The switch to absolute imports and addition of get_test_asset_path improves maintainability and follows the PR's objective of standardizing test asset access.
95-95: LGTM! Consistent test asset path handling
Using get_test_asset_path for test assets improves maintainability and follows the standardization effort.
tests/unit/data_formats/test_common_semantic_segmentation_format.py (2)
17-18: LGTM! Standardized imports
The switch to absolute imports and addition of get_test_asset_path maintains consistency with the project's standardization effort.
21-25: LGTM! Clear and consistent test asset path handling
The use of get_test_asset_path for both dataset paths improves maintainability. The multi-line formatting for the non-standard dataset path enhances readability.
tests/integration/cli/test_detect_format.py (2)
16-16: LGTM! Consistent import addition
The addition of get_test_asset_path aligns with the project's standardization effort.
20-22: LGTM! Standardized test asset paths
The consistent use of get_test_asset_path for all dataset directories improves maintainability and follows the standardization effort.
tests/integration/cli/test_kitti_raw_format.py (1)
9-9: LGTM: Import change aligns with project standards
The change from relative to absolute import improves clarity and maintainability.
tests/unit/test_imagenet_format.py (1)
14-15: LGTM: Consistent implementation of test asset handling
The changes align with the PR objectives:
- Absolute imports improve clarity
- Using
get_test_asset_pathstandardizes asset access across tests
Also applies to: 119-119
tests/unit/test_align_celeba_format.py (1)
19-20: LGTM: Consistent implementation of imports and asset handling
The changes align with the PR objectives and maintain consistency across the test suite.
Also applies to: 23-23
tests/unit/test_celeba_format.py (2)
20-21: LGTM! Import changes improve maintainability.
The switch to absolute imports and addition of get_test_asset_path aligns with the PR objectives and Python's import best practices.
24-25: LGTM! Path handling improvements.
Using get_test_asset_path centralizes test asset path management, making it more maintainable and consistent across tests.
tests/integration/cli/test_patch.py (1)
12-12: LGTM! Consistent import style.
The change to absolute import maintains consistency with other test files and improves clarity.
tests/unit/test_ade20k2020_format.py (1)
17-17: LGTM! The changes improve test asset management.
The updates to import statements and dataset paths using get_test_asset_path enhance maintainability by providing a consistent way to handle test assets across the test suite.
Also applies to: 20-20, 22-22
tests/unit/test_mpii_format.py (1)
17-18: LGTM! Changes maintain consistency with the codebase.
The updates to import statements and dataset path definition follow the same pattern as other test files, ensuring a uniform approach to test asset management.
Also applies to: 21-21
tests/unit/test_mnist_format.py (2)
13-14: LGTM! Import changes improve maintainability.
The switch from relative to absolute imports and the addition of the get_test_asset_path utility aligns with the PR's goal of standardizing test asset handling.
193-193: LGTM! Path handling improvement.
Using get_test_asset_path centralizes asset path management and makes the code more maintainable.
tests/unit/test_mnist_csv_format.py (2)
13-14: LGTM! Consistent import changes.
The import modifications follow the same pattern as other test files, improving consistency across the test suite.
193-193: LGTM! Consistent path handling.
The use of get_test_asset_path for "mnist_csv_dataset" follows the established pattern.
tests/unit/test_market1501_format.py (2)
12-13: LGTM! Consistent import structure.
The import changes maintain consistency with the project-wide standardization effort.
212-212: LGTM! Standardized asset path handling.
The use of get_test_asset_path for "market1501_dataset" follows the established pattern.
tests/unit/test_mots_format.py (1)
14-15: LGTM! Import changes improve maintainability.
The changes to absolute imports and introduction of get_test_asset_path align with best practices for test organization.
tests/unit/test_mot_format.py (2)
14-15: LGTM! Import changes improve maintainability.
The changes to absolute imports and introduction of get_test_asset_path align with best practices for test organization.
310-311: LGTM! Path construction is now standardized.
Using get_test_asset_path improves maintainability by centralizing test asset path management.
tests/unit/test_vgg_face2_format.py (2)
13-14: LGTM! Import changes improve maintainability.
The changes to absolute imports and introduction of get_test_asset_path align with best practices for test organization.
278-278: LGTM! Path construction is now standardized.
Using get_test_asset_path improves maintainability by centralizing test asset path management.
tests/unit/test_cifar_format.py (3)
15-16: LGTM! Import statements are well organized.
The changes standardize the imports by using absolute paths and add the new asset path utility.
263-263: LGTM! Asset path is properly defined.
The constant correctly uses get_test_asset_path for consistent asset management.
265-265: LGTM! Asset path is properly defined.
The constant correctly uses get_test_asset_path for consistent asset management.
tests/unit/test_lfw_format.py (2)
15-16: LGTM! Import statements are well organized.
The changes maintain consistency with the standardized import approach.
299-299: LGTM! Asset path is properly defined.
The constant correctly uses get_test_asset_path for consistent asset management.
tests/unit/test_labelme_format.py (2)
15-16: LGTM! Import statements are well organized.
The changes maintain consistency with the standardized import approach across the test suite.
322-322: LGTM! Asset path is properly defined.
The constant correctly uses get_test_asset_path for consistent asset management.
tests/unit/test_mpii_json_format.py (2)
17-18: LGTM: Import statements updated for better maintainability
The change from relative imports to absolute imports and the addition of the asset utility import aligns with the PR objectives of improving test file organization.
21-25: LGTM: Improved test asset path handling
Using get_test_asset_path instead of direct path construction improves maintainability and consistency across tests.
tests/unit/test_widerface_format.py (2)
14-15: LGTM: Consistent import statement updates
The import changes align with the project-wide standardization of imports and asset handling.
403-403: LGTM: Standardized test asset path handling
The update to use get_test_asset_path maintains consistency with other test files.
tests/unit/test_camvid_format.py (2)
18-19: LGTM: Standardized import handling
The import changes maintain consistency with other test files in the project.
50-50: LGTM: Unified test asset path handling
The update to use get_test_asset_path aligns with the project-wide standardization of test asset management.
tests/unit/test_icdar_format.py (2)
23-24: LGTM! Import statements are well organized.
The change from relative to absolute imports improves clarity and maintainability.
27-27: LGTM! Asset path handling is standardized.
Using get_test_asset_path centralizes test asset management and improves maintainability.
tests/unit/data_formats/test_mapillary_vistas_format.py (2)
21-22: LGTM! Import statements are consistent with project standards.
The change to absolute imports aligns with the project-wide standardization effort.
25-28: LGTM! Dataset path definitions are well structured.
The use of get_test_asset_path with proper formatting improves code organization and maintainability.
tests/unit/data_formats/datumaro/test_datumaro_format.py (2)
33-34: LGTM! Import statements follow project conventions.
The change to absolute imports maintains consistency with the project-wide standardization.
206-206: LGTM! Dataset path handling is standardized.
The use of get_test_asset_path maintains consistency with the project's asset management approach.
Also applies to: 257-257
tests/unit/test_cityscapes_format.py (2)
22-23: LGTM! Import statements are well organized.
The change from relative to absolute imports improves clarity and maintainability.
26-27: LGTM! Standardized asset path handling.
Using get_test_asset_path improves maintainability by centralizing path management logic.
tests/unit/test_transforms.py (1)
29-29: LGTM! Import statement is properly updated.
The change to absolute imports maintains consistency with the project's import style.
tests/unit/test_kitti_format.py (2)
29-30: LGTM! Import statements are well structured.
The change to absolute imports and addition of get_test_asset_path import maintains consistency across the test suite.
33-33: LGTM! Standardized asset path handling.
Using get_test_asset_path improves maintainability by centralizing path management logic.
|
|
||
| from .requirements import Requirements, mark_requirement | ||
|
|
||
| from tests.requirements import Requirements, mark_requirement | ||
| from tests.utils.assets import get_test_asset_path |
There was a problem hiding this comment.
💡 Codebase verification
Path handling inconsistencies found in integration tests
The verification reveals that while unit tests consistently use get_test_asset_path, there are inconsistencies in integration tests. Specifically:
tests/integration/cli/test_voc_format.pyuses manual path construction for both VOC and YOLO dataset pathstests/integration/cli/test_sly_point_cloud_format.pyuses manual path construction for pointcloud dataset path
These files should be updated to use the standardized get_test_asset_path utility, which is already properly implemented in tests/utils/assets.py and widely used across unit tests.
🔗 Analysis chain
Verify consistent path handling across test files
Let's ensure all test files follow the same pattern for asset path handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for inconsistent path handling in test files
# Find test files still using manual path construction
echo "Files potentially needing path handling updates:"
rg -l "osp\.join.*tests.*assets" "tests/**/*.py"
# Find test files already using get_test_asset_path
echo -e "\nFiles already using standardized path handling:"
rg -l "get_test_asset_path" "tests/**/*.py"
Length of output: 420
Script:
#!/bin/bash
# Description: Check for inconsistent path handling in test files using correct path patterns
# Find test files still using manual path construction
echo "Files potentially needing path handling updates:"
rg -l "osp\.join.*assets" "tests/"
# Find test files already using get_test_asset_path
echo -e "\nFiles already using standardized path handling:"
rg -l "get_test_asset_path" "tests/"
# List all Python test files for reference
echo -e "\nAll Python test files in the repository:"
fd "test.*\.py$" "tests/"
Length of output: 5339
Script:
#!/bin/bash
# Let's examine the actual usage in the files that show manual path construction
echo "Checking manual path construction in tests/utils/assets.py:"
rg "osp\.join.*assets" tests/utils/assets.py -B 2 -A 2
echo -e "\nChecking manual path construction in tests/integration/cli/test_voc_format.py:"
rg "osp\.join.*assets" tests/integration/cli/test_voc_format.py -B 2 -A 2
echo -e "\nChecking manual path construction in tests/integration/cli/test_sly_point_cloud_format.py:"
rg "osp\.join.*assets" tests/integration/cli/test_sly_point_cloud_format.py -B 2 -A 2
Length of output: 1296
|
There is no new code. |
Summary
get_test_asset_pathto access assets everywhere in testsHow to test
Checklist
developbranchLicense
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
Requirementsandmark_requirementacross multiple test files.get_test_asset_pathin various test files to enhance maintainability and clarity.