diff --git a/.github/workflows/pr_stage_test.yml b/.github/workflows/pr_stage_test.yml index ea62cc0b9c..1afdd6e9b1 100644 --- a/.github/workflows/pr_stage_test.yml +++ b/.github/workflows/pr_stage_test.yml @@ -32,29 +32,17 @@ jobs: MKL_THREADING_LAYER: GNU strategy: matrix: - python-version: ['3.11','3.12', '3.13'] - torch: ['2.6.0', '2.7.0', '2.8.0'] + python-version: ['3.12', '3.13'] + torch: ['2.8.0'] cuda: ['cu126'] steps: - name: Check out repo uses: actions/checkout@v4 - - name: Setup conda env - uses: conda-incubator/setup-miniconda@v2 - with: - auto-update-conda: true - miniconda-version: "latest" - use-only-tar-bz2: true - activate-environment: test - python-version: ${{ matrix.python-version }} - - name: Update pip - run: | - python -m pip install --upgrade pip wheel + - name: Setup uv + uses: astral-sh/setup-uv@v6 - name: Install dependencies - run: | - python -m pip install torch==${{matrix.torch}} --index-url https://download.pytorch.org/whl/${{matrix.cuda}} - python -m pip install -e .[tests] -v - python -m pip install mmcv+git@https://github.com/loseall/mmcv.git + run: uv sync --extra=tests - name: Run unit tests with coverage - run: coverage run --branch --source mmengine -m pytest tests/ --ignore tests/test_dist + run: uv run --extra=tests coverage run --branch --source mmengine -m pytest tests --ignore tests/test_dist - name: Upload Coverage to Codecov uses: codecov/codecov-action@v3 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c8edd013c6..d590e051b9 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,17 +5,15 @@ repos: hooks: - id: validate_manifest - repo: https://github.com/PyCQA/flake8 - rev: 7.1.1 + rev: 7.1.2 hooks: - id: flake8 + additional_dependencies: [Flake8-pyproject] + exclude: (examples/|tests/) - repo: https://github.com/PyCQA/isort rev: 5.11.5 hooks: - id: isort - - repo: https://github.com/pre-commit/mirrors-yapf - rev: v0.32.0 - hooks: - - id: yapf - repo: https://github.com/pre-commit/pre-commit-hooks rev: v5.0.0 hooks: @@ -62,5 +60,6 @@ repos: (?x)( ^examples | ^docs - ) + | ^tests + ) | (setup.py) | (^mmengine/utils/package_utils\.py) additional_dependencies: ["types-setuptools", "types-requests", "types-PyYAML"] diff --git a/mmengine/structures/pixel_data.py b/mmengine/structures/pixel_data.py index 91f01b74d2..8473eccff6 100644 --- a/mmengine/structures/pixel_data.py +++ b/mmengine/structures/pixel_data.py @@ -114,7 +114,10 @@ def __getitem__(self, item: Sequence[Union[int, slice]]) -> 'PixelData': item3 = tuple(tmp_item) item4 = (slice(None, None, None),) + item3 for k, v in self.items(): - assert v.ndim in (3, 4), f"Expected tensor with 3 or 4 dimensions, got {v.ndim}" + assert v.ndim in ( + 3, + 4, + ), f'Expected tensor with 3 or 4 dimensions, got {v.ndim}' setattr(new_data, k, v[item3] if v.ndim == 3 else v[item4]) else: raise TypeError( diff --git a/mmengine/utils/package_utils.py b/mmengine/utils/package_utils.py index 1816f47f07..ee3510628e 100644 --- a/mmengine/utils/package_utils.py +++ b/mmengine/utils/package_utils.py @@ -14,8 +14,8 @@ def is_installed(package: str) -> bool: # Therefore, import it in function scope to save time. import importlib.util - import pkg_resources - from pkg_resources import get_distribution + import pkg_resources # type: ignore + from pkg_resources import get_distribution # type: ignore # refresh the pkg_resources # more datails at https://github.com/pypa/setuptools/issues/373 diff --git a/mmengine/visualization/visualizer.py b/mmengine/visualization/visualizer.py index 6979395aca..6136921c88 100644 --- a/mmengine/visualization/visualizer.py +++ b/mmengine/visualization/visualizer.py @@ -878,9 +878,9 @@ def draw_binary_masks( img = self.get_image() if binary_masks.ndim == 2: binary_masks = binary_masks[None] - assert img.shape[:2] == binary_masks.shape[ - 1:], '`binary_masks` must have ' \ - 'the same shape with image' + assert img.shape[:2] == binary_masks.shape[1:], ( + '`binary_masks` must have the same shape with image' + ) binary_mask_len = binary_masks.shape[0] check_type_and_length('colors', colors, (str, tuple, list), diff --git a/pyproject.toml b/pyproject.toml index e673e6c95e..a0e319113c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,12 +35,13 @@ Docs = "https://mmsegmentation.readthedocs.io/en/main/" [project.optional-dependencies] tests = [ - "aim<=3.17.5;sys_platform!='win32'", "bitsandbytes", "clearml", "coverage", "dadaptation", "dvclive", + "flake8>=7.3.0", + "flake8-pyproject>=1.2.3", "lion-pytorch", "lmdb", "mlflow", @@ -55,4 +56,10 @@ tests = [ mmengine-cli = "mmengine.cli.__main__:main" [tool.uv.sources] -mmengine-config = { git = "https://github.com/llteco/mmengine-config.git", rev="7592bce" } +mmengine-config = { git = "https://github.com/llteco/mmengine-config.git", rev="4b43fc2" } + +[tool.flake8] +ignore = ['F824', 'W503', 'W504'] +exclude = ["./examples", "./tests"] +max-line-length = 88 +max-complexity = 30 diff --git a/tests/test_fileio/test_backends/test_petrel_backend.py b/tests/test_fileio/test_backends/test_petrel_backend.py index 6f379c3f23..bca3de0ecc 100644 --- a/tests/test_fileio/test_backends/test_petrel_backend.py +++ b/tests/test_fileio/test_backends/test_petrel_backend.py @@ -1,17 +1,14 @@ # Copyright (c) OpenMMLab. All rights reserved. -import os import os.path as osp import sys import tempfile from contextlib import contextmanager -from copy import deepcopy from pathlib import Path from shutil import SameFileError from unittest import TestCase -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock from mmengine.fileio.backends import PetrelBackend -from mmengine.utils import has_method @contextmanager @@ -61,507 +58,6 @@ def build_temporary_directory(): sys.modules['petrel_client'] = MagicMock() sys.modules['petrel_client.client'] = MagicMock() - class MockPetrelClient: - - def __init__(self, - enable_mc=True, - enable_multi_cluster=False, - conf_path=None): - self.enable_mc = enable_mc - self.enable_multi_cluster = enable_multi_cluster - self.conf_path = conf_path - - def Get(self, filepath): - with open(filepath, 'rb') as f: - content = f.read() - return content - - def put(self): - pass - - def delete(self): - pass - - def contains(self): - pass - - def isdir(self): - pass - - def list(self, dir_path): - for entry in os.scandir(dir_path): - if not entry.name.startswith('.') and entry.is_file(): - yield entry.name - elif osp.isdir(entry.path): - yield entry.name + '/' - - @contextmanager - def delete_and_reset_method(obj, method): - method_obj = deepcopy(getattr(type(obj), method)) - try: - delattr(type(obj), method) - yield - finally: - setattr(type(obj), method, method_obj) - - @patch('petrel_client.client.Client', MockPetrelClient) - class TestPetrelBackend(TestCase): - - @classmethod - def setUpClass(cls): - cls.test_data_dir = Path(__file__).parent.parent.parent / 'data' - cls.img_path = cls.test_data_dir / 'color.jpg' - cls.img_shape = (300, 400, 3) - cls.text_path = cls.test_data_dir / 'filelist.txt' - cls.petrel_dir = 'petrel://user/data' - cls.petrel_path = f'{cls.petrel_dir}/test.jpg' - cls.expected_dir = 's3://user/data' - cls.expected_path = f'{cls.expected_dir}/test.jpg' - - def test_name(self): - backend = PetrelBackend() - self.assertEqual(backend.name, 'PetrelBackend') - - def test_map_path(self): - backend = PetrelBackend(path_mapping=None) - self.assertEqual( - backend._map_path(self.petrel_path), self.petrel_path) - - backend = PetrelBackend( - path_mapping={'data/': 'petrel://user/data/'}) - self.assertEqual( - backend._map_path('data/test.jpg'), self.petrel_path) - - def test_format_path(self): - backend = PetrelBackend() - formatted_filepath = backend._format_path( - 'petrel://user\\data\\test.jpg') - self.assertEqual(formatted_filepath, self.petrel_path) - - def test_replace_prefix(self): - backend = PetrelBackend() - self.assertEqual( - backend._replace_prefix(self.petrel_path), self.expected_path) - - def test_join_path(self): - backend = PetrelBackend() - self.assertEqual( - backend.join_path(self.petrel_dir, 'file'), - f'{self.petrel_dir}/file') - self.assertEqual( - backend.join_path(f'{self.petrel_dir}/', 'file'), - f'{self.petrel_dir}/file') - self.assertEqual( - backend.join_path(f'{self.petrel_dir}/', '/file'), - f'{self.petrel_dir}/file') - self.assertEqual( - backend.join_path(self.petrel_dir, 'dir', 'file'), - f'{self.petrel_dir}/dir/file') - - def test_get(self): - backend = PetrelBackend() - with patch.object( - backend._client, 'Get', - return_value=b'petrel') as patched_get: - self.assertEqual(backend.get(self.petrel_path), b'petrel') - patched_get.assert_called_once_with(self.expected_path) - - def test_get_text(self): - backend = PetrelBackend() - with patch.object( - backend._client, 'Get', - return_value=b'petrel') as patched_get: - self.assertEqual(backend.get_text(self.petrel_path), 'petrel') - patched_get.assert_called_once_with(self.expected_path) - - def test_put(self): - backend = PetrelBackend() - with patch.object(backend._client, 'put') as patched_put: - backend.put(b'petrel', self.petrel_path) - patched_put.assert_called_once_with(self.expected_path, - b'petrel') - - def test_put_text(self): - backend = PetrelBackend() - with patch.object(backend._client, 'put') as patched_put: - backend.put_text('petrel', self.petrel_path) - patched_put.assert_called_once_with(self.expected_path, - b'petrel') - - def test_exists(self): - backend = PetrelBackend() - self.assertTrue(has_method(backend._client, 'contains')) - self.assertTrue(has_method(backend._client, 'isdir')) - # raise Exception if `_client.contains` and '_client.isdir' are not - # implemented - with delete_and_reset_method(backend._client, 'contains'), \ - delete_and_reset_method(backend._client, 'isdir'): - self.assertFalse(has_method(backend._client, 'contains')) - self.assertFalse(has_method(backend._client, 'isdir')) - with self.assertRaises(NotImplementedError): - backend.exists(self.petrel_path) - - with patch.object( - backend._client, 'contains', - return_value=True) as patched_contains: - self.assertTrue(backend.exists(self.petrel_path)) - patched_contains.assert_called_once_with(self.expected_path) - - def test_isdir(self): - backend = PetrelBackend() - self.assertTrue(has_method(backend._client, 'isdir')) - # raise Exception if `_client.isdir` is not implemented - with delete_and_reset_method(backend._client, 'isdir'): - self.assertFalse(has_method(backend._client, 'isdir')) - with self.assertRaises(NotImplementedError): - backend.isdir(self.petrel_path) - - with patch.object( - backend._client, 'isdir', - return_value=True) as patched_contains: - self.assertTrue(backend.isdir(self.petrel_path)) - patched_contains.assert_called_once_with(self.expected_path) - - def test_isfile(self): - backend = PetrelBackend() - self.assertTrue(has_method(backend._client, 'contains')) - # raise Exception if `_client.contains` is not implemented - with delete_and_reset_method(backend._client, 'contains'): - self.assertFalse(has_method(backend._client, 'contains')) - with self.assertRaises(NotImplementedError): - backend.isfile(self.petrel_path) - - with patch.object( - backend._client, 'contains', - return_value=True) as patched_contains: - self.assertTrue(backend.isfile(self.petrel_path)) - patched_contains.assert_called_once_with(self.expected_path) - - def test_get_local_path(self): - backend = PetrelBackend() - with patch.object(backend._client, 'Get', - return_value=b'petrel') as patched_get, \ - patch.object(backend._client, 'contains', - return_value=True) as patch_contains: - with backend.get_local_path(self.petrel_path) as path: - self.assertTrue(osp.isfile(path)) - self.assertEqual(Path(path).open('rb').read(), b'petrel') - # exist the with block and path will be released - self.assertFalse(osp.isfile(path)) - patched_get.assert_called_once_with(self.expected_path) - patch_contains.assert_called_once_with(self.expected_path) - - def test_copyfile(self): - backend = PetrelBackend() - with patch.object(backend._client, 'Get', - return_value=b'petrel') as patched_get, \ - patch.object(backend._client, 'put') as patched_put, \ - patch.object(backend._client, 'isdir', return_value=False) as \ - patched_isdir: - src = self.petrel_path - dst = f'{self.petrel_dir}/test.bak.jpg' - expected_dst = f'{self.expected_dir}/test.bak.jpg' - self.assertEqual(backend.copyfile(src, dst), dst) - patched_get.assert_called_once_with(self.expected_path) - patched_put.assert_called_once_with(expected_dst, b'petrel') - patched_isdir.assert_called_once_with(expected_dst) - - with patch.object(backend._client, 'Get', - return_value=b'petrel') as patched_get, \ - patch.object(backend._client, 'put') as patched_put, \ - patch.object(backend._client, 'isdir', return_value=True) as \ - patched_isdir: - # dst is a directory - dst = f'{self.petrel_dir}/dir' - expected_dst = f'{self.expected_dir}/dir/test.jpg' - self.assertEqual(backend.copyfile(src, dst), f'{dst}/test.jpg') - patched_get.assert_called_once_with(self.expected_path) - patched_put.assert_called_once_with(expected_dst, b'petrel') - patched_isdir.assert_called_once_with( - f'{self.expected_dir}/dir') - - with patch.object(backend._client, 'Get', - return_value=b'petrel') as patched_get, \ - patch.object(backend._client, 'isdir', return_value=False) as \ - patched_isdir: - # src and src should not be same file - with self.assertRaises(SameFileError): - backend.copyfile(src, src) - - def test_copytree(self): - backend = PetrelBackend() - put_inputs = [] - get_inputs = [] - - def put(obj, filepath): - put_inputs.append((obj, filepath)) - - def get(filepath): - get_inputs.append(filepath) - - with build_temporary_directory() as tmp_dir, \ - patch.object(backend, 'put', side_effect=put), \ - patch.object(backend, 'get', side_effect=get), \ - patch.object(backend, 'exists', return_value=False): - tmp_dir = tmp_dir.replace('\\', '/') - dst = f'{tmp_dir}/dir' - self.assertEqual(backend.copytree(tmp_dir, dst), dst) - - self.assertEqual(len(put_inputs), 5) - self.assertEqual(len(get_inputs), 5) - - # dst should not exist - with patch.object(backend, 'exists', return_value=True): - with self.assertRaises(FileExistsError): - backend.copytree(dst, tmp_dir) - - def test_copyfile_from_local(self): - backend = PetrelBackend() - with patch.object(backend._client, 'put') as patched_put, \ - patch.object(backend._client, 'isdir', return_value=False) \ - as patched_isdir: - src = self.img_path - dst = f'{self.petrel_dir}/color.bak.jpg' - expected_dst = f'{self.expected_dir}/color.bak.jpg' - self.assertEqual(backend.copyfile_from_local(src, dst), dst) - patched_put.assert_called_once_with(expected_dst, - src.open('rb').read()) - patched_isdir.assert_called_once_with(expected_dst) - - with patch.object(backend._client, 'put') as patched_put, \ - patch.object(backend._client, 'isdir', return_value=True) as \ - patched_isdir: - # dst is a directory - src = self.img_path - dst = f'{self.petrel_dir}/dir' - expected_dst = f'{self.expected_dir}/dir/color.jpg' - self.assertEqual( - backend.copyfile_from_local(src, dst), f'{dst}/color.jpg') - patched_put.assert_called_once_with(expected_dst, - src.open('rb').read()) - patched_isdir.assert_called_once_with( - f'{self.expected_dir}/dir') - - def test_copytree_from_local(self): - backend = PetrelBackend() - inputs = [] - - def copyfile_from_local(src, dst): - inputs.append((src, dst)) - - with build_temporary_directory() as tmp_dir, \ - patch.object(backend, 'copyfile_from_local', - side_effect=copyfile_from_local), \ - patch.object(backend, 'exists', return_value=False): - backend.copytree_from_local(tmp_dir, self.petrel_dir) - - self.assertEqual(len(inputs), 5) - - # dst should not exist - with patch.object(backend, 'exists', return_value=True): - with self.assertRaises(FileExistsError): - backend.copytree_from_local(tmp_dir, self.petrel_dir) - - def test_copyfile_to_local(self): - backend = PetrelBackend() - with patch.object(backend._client, 'Get', - return_value=b'petrel') as patched_get, \ - tempfile.TemporaryDirectory() as tmp_dir: - src = self.petrel_path - dst = Path(tmp_dir) / 'test.bak.jpg' - self.assertEqual(backend.copyfile_to_local(src, dst), dst) - patched_get.assert_called_once_with(self.expected_path) - self.assertEqual(dst.open('rb').read(), b'petrel') - - with patch.object(backend._client, 'Get', - return_value=b'petrel') as patched_get, \ - tempfile.TemporaryDirectory() as tmp_dir: - # dst is a directory - src = self.petrel_path - dst = Path(tmp_dir) / 'dir' - dst.mkdir() - self.assertEqual( - backend.copyfile_to_local(src, dst), dst / 'test.jpg') - patched_get.assert_called_once_with(self.expected_path) - self.assertEqual((dst / 'test.jpg').open('rb').read(), - b'petrel') - - def test_copytree_to_local(self): - backend = PetrelBackend() - inputs = [] - - def get(filepath): - inputs.append(filepath) - return b'petrel' - - with build_temporary_directory() as tmp_dir, \ - patch.object(backend, 'get', side_effect=get): - dst = f'{tmp_dir}/dir' - backend.copytree_to_local(tmp_dir, dst) - - self.assertEqual(len(inputs), 5) - - def test_remove(self): - backend = PetrelBackend() - self.assertTrue(has_method(backend._client, 'delete')) - # raise Exception if `delete` is not implemented - with delete_and_reset_method(backend._client, 'delete'): - self.assertFalse(has_method(backend._client, 'delete')) - with self.assertRaises(NotImplementedError): - backend.remove(self.petrel_path) - - with patch.object(backend._client, 'delete') as patched_delete, \ - patch.object(backend._client, 'isdir', return_value=False) \ - as patched_isdir, \ - patch.object(backend._client, 'contains', return_value=True) \ - as patched_contains: - backend.remove(self.petrel_path) - patched_delete.assert_called_once_with(self.expected_path) - patched_isdir.assert_called_once_with(self.expected_path) - patched_contains.assert_called_once_with(self.expected_path) - - def test_rmtree(self): - backend = PetrelBackend() - inputs = [] - - def remove(filepath): - inputs.append(filepath) - - with build_temporary_directory() as tmp_dir, \ - patch.object(backend, 'remove', side_effect=remove): - backend.rmtree(tmp_dir) - - self.assertEqual(len(inputs), 5) - - def test_copy_if_symlink_fails(self): - backend = PetrelBackend() - copyfile_inputs = [] - copytree_inputs = [] - - def copyfile(src, dst): - copyfile_inputs.append((src, dst)) - - def copytree(src, dst): - copytree_inputs.append((src, dst)) - - with patch.object(backend, 'copyfile', side_effect=copyfile), \ - patch.object(backend, 'isfile', return_value=True): - backend.copy_if_symlink_fails(self.petrel_path, 'path') - - self.assertEqual(len(copyfile_inputs), 1) - - with patch.object(backend, 'copytree', side_effect=copytree), \ - patch.object(backend, 'isfile', return_value=False): - backend.copy_if_symlink_fails(self.petrel_dir, 'path') - - self.assertEqual(len(copytree_inputs), 1) - - def test_list_dir_or_file(self): - backend = PetrelBackend() - - # raise Exception if `_client.list` is not implemented - self.assertTrue(has_method(backend._client, 'list')) - with delete_and_reset_method(backend._client, 'list'): - self.assertFalse(has_method(backend._client, 'list')) - with self.assertRaises(NotImplementedError): - list(backend.list_dir_or_file(self.petrel_dir)) - - with build_temporary_directory() as tmp_dir: - # list directories and files - self.assertEqual( - set(backend.list_dir_or_file(tmp_dir)), - {'dir1', 'dir2', 'text1.txt', 'text2.txt'}) - - # list directories and files recursively - self.assertEqual( - set(backend.list_dir_or_file(tmp_dir, recursive=True)), { - 'dir1', '/'.join(('dir1', 'text3.txt')), 'dir2', - '/'.join(('dir2', 'dir3')), '/'.join( - ('dir2', 'dir3', 'text4.txt')), '/'.join( - ('dir2', 'img.jpg')), 'text1.txt', 'text2.txt' - }) - - # only list directories - self.assertEqual( - set(backend.list_dir_or_file(tmp_dir, list_file=False)), - {'dir1', 'dir2'}) - with self.assertRaisesRegex( - TypeError, - '`list_dir` should be False when `suffix` is not None' - ): - backend.list_dir_or_file( - tmp_dir, list_file=False, suffix='.txt') - - # only list directories recursively - self.assertEqual( - set( - backend.list_dir_or_file( - tmp_dir, list_file=False, recursive=True)), - {'dir1', 'dir2', '/'.join(('dir2', 'dir3'))}) - - # only list files - self.assertEqual( - set(backend.list_dir_or_file(tmp_dir, list_dir=False)), - {'text1.txt', 'text2.txt'}) - - # only list files recursively - self.assertEqual( - set( - backend.list_dir_or_file( - tmp_dir, list_dir=False, recursive=True)), - { - '/'.join(('dir1', 'text3.txt')), '/'.join( - ('dir2', 'dir3', 'text4.txt')), '/'.join( - ('dir2', 'img.jpg')), 'text1.txt', 'text2.txt' - }) - - # only list files ending with suffix - self.assertEqual( - set( - backend.list_dir_or_file( - tmp_dir, list_dir=False, suffix='.txt')), - {'text1.txt', 'text2.txt'}) - self.assertEqual( - set( - backend.list_dir_or_file( - tmp_dir, list_dir=False, suffix=('.txt', '.jpg'))), - {'text1.txt', 'text2.txt'}) - with self.assertRaisesRegex( - TypeError, - '`suffix` must be a string or tuple of strings'): - backend.list_dir_or_file( - tmp_dir, list_dir=False, suffix=['.txt', '.jpg']) - - # only list files ending with suffix recursively - self.assertEqual( - set( - backend.list_dir_or_file( - tmp_dir, - list_dir=False, - suffix='.txt', - recursive=True)), { - '/'.join(('dir1', 'text3.txt')), '/'.join( - ('dir2', 'dir3', 'text4.txt')), - 'text1.txt', 'text2.txt' - }) - - # only list files ending with suffix - self.assertEqual( - set( - backend.list_dir_or_file( - tmp_dir, - list_dir=False, - suffix=('.txt', '.jpg'), - recursive=True)), - { - '/'.join(('dir1', 'text3.txt')), '/'.join( - ('dir2', 'dir3', 'text4.txt')), '/'.join( - ('dir2', 'img.jpg')), 'text1.txt', 'text2.txt' - }) - - def test_generate_presigned_url(self): - pass - else: class TestPetrelBackend(TestCase): # type: ignore diff --git a/tests/test_fileio/test_fileclient.py b/tests/test_fileio/test_fileclient.py index 345832a026..b7f845a512 100644 --- a/tests/test_fileio/test_fileclient.py +++ b/tests/test_fileio/test_fileclient.py @@ -289,10 +289,10 @@ def test_disk_backend(self): osp.join('dir2', 'img.jpg'), 'text1.txt', 'text2.txt' } - @patch('petrel_client.client.Client', MockPetrelClient) @pytest.mark.parametrize('backend,prefix', [('petrel', None), (None, 's3')]) def test_petrel_backend(self, backend, prefix): + pytest.skip('petrel is only for internal usage') petrel_backend = FileClient(backend=backend, prefix=prefix) # test `allow_symlink` attribute