-
Notifications
You must be signed in to change notification settings - Fork 1
Update all pre-commit hook version and fix the resulting errors #24
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: main
Are you sure you want to change the base?
Conversation
… longer maintained and not support mdformat rev 1.0.0.
…"SimplePath", "str" [call-overload]
…rgument type "InstanceData" [operator]
…h"; expected "str" [arg-type]
Fix mmengine/runner/runner.py:1439: error: Argument "num_workers" to "worker_init_fn" has incompatible type "Any | None"; expected "int" [arg-type].
…llable [misc]. Fix mypy mmengine/runner/_flexible_runner.py:877: error: Argument "num_workers" to "worker_init_fn" has incompatible type "Any | None"; expected "int" [arg-type].
|
There're some pytest failures, I'm trying to figure out what happened. ============================================================== short test summary info ==============================================================
FAILED tests/test_fileio/test_backends/test_local_backend.py::TestLocalBackend::test_copyfile_0 - AssertionError: '/tmp/tmpk0503p6e/test.txt.bak' != PosixPath('/tmp/tmpk0503p6e/test.txt.bak')
FAILED tests/test_fileio/test_backends/test_local_backend.py::TestLocalBackend::test_copyfile_from_local_0 - AssertionError: '/tmp/tmpaqcephbs/test.txt.bak' != PosixPath('/tmp/tmpaqcephbs/test.txt.bak')
FAILED tests/test_fileio/test_backends/test_local_backend.py::TestLocalBackend::test_copyfile_to_local_0 - AssertionError: '/tmp/tmpjjlhckku/test.txt.bak' != PosixPath('/tmp/tmpjjlhckku/test.txt.bak')
FAILED tests/test_fileio/test_backends/test_local_backend.py::TestLocalBackend::test_copytree_0 - AssertionError: '/tmp/tmpbfgej59f/dir100' != PosixPath('/tmp/tmpbfgej59f/dir100')
FAILED tests/test_fileio/test_backends/test_local_backend.py::TestLocalBackend::test_copytree_from_local_0 - AssertionError: '/tmp/tmplswa4rdf/dir100' != PosixPath('/tmp/tmplswa4rdf/dir100')
FAILED tests/test_fileio/test_backends/test_local_backend.py::TestLocalBackend::test_copytree_to_local_0 - AssertionError: '/tmp/tmpd0zpjgca/dir100' != PosixPath('/tmp/tmpd0zpjgca/dir100')
FAILED tests/test_fileio/test_fileclient.py::TestFileClient::test_http_backend[http-None] - urllib.error.URLError: <urlopen error [Errno 111] Connection refused>
FAILED tests/test_fileio/test_fileclient.py::TestFileClient::test_http_backend[None-http] - urllib.error.URLError: <urlopen error [Errno 111] Connection refused>
FAILED tests/test_fileio/test_io.py::test_copyfile - AssertionError: assert '/tmp/tmp3fiklerp/test.txt.bak' == PosixPath('/tmp/tmp3fiklerp/test.txt.bak')
FAILED tests/test_fileio/test_io.py::test_copytree - AssertionError: assert '/tmp/tmpdajzvqqi/dir100' == PosixPath('/tmp/tmpdajzvqqi/dir100')
FAILED tests/test_fileio/test_io.py::test_copyfile_from_local - AssertionError: assert '/tmp/tmpukbefd3j/test.txt.bak' == PosixPath('/tmp/tmpukbefd3j/test.txt.bak')
FAILED tests/test_fileio/test_io.py::test_copytree_from_local - AssertionError: assert '/tmp/tmpqnmdk6_e/dir100' == PosixPath('/tmp/tmpqnmdk6_e/dir100')
FAILED tests/test_fileio/test_io.py::test_copyfile_to_local - AssertionError: assert '/tmp/tmp7i0xqlhk/test.txt.bak' == PosixPath('/tmp/tmp7i0xqlhk/test.txt.bak')
FAILED tests/test_fileio/test_io.py::test_copytree_to_local - AssertionError: assert '/tmp/tmpu4aegp01/dir100' == PosixPath('/tmp/tmpu4aegp01/dir100')
FAILED tests/test_hooks/test_empty_cache_hook.py::TestEmptyCacheHook::test_with_runner - AssertionError: 5 != 4
FAILED tests/test_strategies/test_fsdp.py::TestStrategy::test_run_strategy - torch.multiprocessing.spawn.ProcessRaisedException:
FAILED tests/test_visualizer/test_vis_backend.py::TestDVCLiveVisBackend::test_close - AttributeError: 'Live' object has no attribute '_dvc_file'. Did you mean: 'dvc_file'?
FAILED tests/test_visualizer/test_vis_backend.py::TestAimVisBackend::test_experiment - ImportError: Please run "pip install aim" to install aim
FAILED tests/test_visualizer/test_vis_backend.py::TestAimVisBackend::test_add_config - ImportError: Please run "pip install aim" to install aim
FAILED tests/test_visualizer/test_vis_backend.py::TestAimVisBackend::test_add_image - ImportError: Please run "pip install aim" to install aim
FAILED tests/test_visualizer/test_vis_backend.py::TestAimVisBackend::test_add_scalar - ImportError: Please run "pip install aim" to install aim
FAILED tests/test_visualizer/test_vis_backend.py::TestAimVisBackend::test_add_scalars - ImportError: Please run "pip install aim" to install aim
FAILED tests/test_visualizer/test_vis_backend.py::TestAimVisBackend::test_close - ImportError: Please run "pip install aim" to install aim
====================================== 23 failed, 34 passed, 167 deselected, 879 warnings in 329.35s (0:05:29) ====================================== |
There're test unit logical modification in this commit.
…o set `val_begin` to infinite instead of `val_interval`. Because the logic of `EpochBasedTrainLoop` will call validate at the end of the last epoch. And this will not happen when `self._epoch < self.val_begin`.
d41c1e8 to
bee0fbe
Compare
…nput type. `test_io` unit should be modified.
|
This PR includes many modifications, but there should be no BC-breaking. Most of the modifications are introduced to make CICD happy. |
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.
Pull Request Overview
This PR applies automated code formatting improvements across multiple files using pre-commit hooks. The changes primarily focus on code style consistency without altering functionality.
- Updated pre-commit hook configurations with newer versions of linting tools
- Reformatted code to comply with line length and style guidelines (removing unnecessary whitespace, adjusting line breaks)
- Enhanced type hints for better clarity
- Fixed documentation formatting (removing extra spaces, correcting escaped characters)
Reviewed Changes
Copilot reviewed 28 out of 41 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.pre-commit-config.yaml |
Updated pre-commit hook versions and configuration (flake8, isort, yapf, mypy, etc.) |
tests/test_optim/test_optimizer/test_optimizer_wrapper.py |
Reformatted conditional expressions for better readability |
tests/test_hooks/test_empty_cache_hook.py |
Updated config attribute from val_interval to val_begin |
tests/test_fileio/test_io.py |
Changed path concatenation from osp.join to Path division operator |
tests/test_fileio/test_backends/test_local_backend.py |
Updated test expectations to match new path handling implementation |
tests/test_dataset/test_base_dataset.py |
Reformatted comparison expressions across multiple lines |
tests/test_analysis/test_jit_analysis.py |
Reformatted dictionary comprehension for better readability |
mmengine/visualization/visualizer.py |
Reformatted assertion statement with awkward line breaks |
mmengine/utils/package_utils.py |
Added explicit string conversion for path compatibility |
mmengine/utils/dl_utils/torch_ops.py |
Reformatted multi-line assignment for better readability |
mmengine/structures/instance_data.py |
Enhanced type hints using cast and get_args, simplified type aliases |
mmengine/runner/runner.py |
Reformatted function calls and added default parameter value |
mmengine/runner/_flexible_runner.py |
Same reformatting as runner.py |
mmengine/runner/checkpoint.py |
Reformatted dictionary comprehension |
mmengine/model/test_time_aug.py |
Reformatted dictionary comprehension |
mmengine/hooks/checkpoint_hook.py |
Reformatted multi-line assertion |
mmengine/fileio/io.py |
Removed unused global variable declaration |
mmengine/fileio/file_client.py |
Reformatted function signature line break |
mmengine/fileio/backends/registry_utils.py |
Removed unused global variable declaration |
mmengine/fileio/backends/local_backend.py |
Enhanced return type annotations and implementation to preserve Path types |
mmengine/dataset/utils.py |
Reformatted dictionary comprehension |
mmengine/config/config.py |
Reformatted dictionary comprehensions |
mmengine/_strategy/deepspeed.py |
Reformatted multi-line assertion |
| Multiple documentation files | Fixed spacing issues, corrected escaped markdown characters, reformatted tables |
Comments suppressed due to low confidence (1)
mmengine/config/config.py:270
- ConfigDict.reduce_ex returns tuple of size 5 and tuple of size 6.
def __reduce_ex__(self, proto):
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def join_path(self, filepath: Union[str, Path], *filepaths: | ||
| Union[str, Path]) -> str: |
Copilot
AI
Nov 8, 2025
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.
The return type annotation str in FileClient.join_path is inconsistent with the updated LocalBackend.join_path which now returns Union[str, Path]. This creates an API inconsistency where the backend implementation returns a different type than what the client interface declares. The return type should be updated to Union[str, Path] to match the backend implementation.
| assert (bboxes[:, 0] <= bboxes[:, 2]).all() and (bboxes[:, 1] | ||
| <= bboxes[:, | ||
| 3]).all() |
Copilot
AI
Nov 8, 2025
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.
[nitpick] The line break placement creates awkward formatting where bboxes[:, and 3] are split across lines. Consider reformatting to either keep the array indexing on one line or break at a more natural boundary, such as: assert (bboxes[:, 0] <= bboxes[:, 2]).all() and (bboxes[:, 1] <= bboxes[:, 3]).all() on a single line or breaking before the and operator.
…difies several markdown.
Motivation
As mentioned in #23, the pre-commit hooks' version are out-of-date. So doing
pre-commit autoupdatefor this repo. This introduces many lint errors and static check fails. This PR also fixes them.pre-commit Upgrade
The following hooks' have been upgraded:
v4.0.0->v4.3.07.1.1->7.3.05.11.5->7.0.0v0.32.0->v0.43.0v5.0.0->v6.0.00.7.9->1.0.006907d0->v1.7.7v3.0.0->v3.21.0v1.2.0->v1.18.20.9.5->0.9.7Remove
mdformat-openmmlabandmdformat_frontmatteras they are no longer maintained and not supportmdformat=1.0.0.fix-encoding-pragmahas been removed aspyupgradenow provides the same effect.flake8 now operates with
--max-line-length=90to avoid too much auto lint modifications.Lint Auto Fixes
New
flake8Reports SeveralF824Errorsglobal name/nonlocal nameis unused: name is never assigned in scope.mmengine/fileio/backends/registry_utils.pyandmmengine/fileio/io.pyare modified to fix this error.When locally modifying a global dict, it's not necessary to manually declare that the dict is a global variable, as it won't create a new local variable.
Fix
mypyErrorsNo overload variant of "join" matches argument types "SimplePath", "str" [call-overload]
mmengine/utils/package_utils.py,osp.joinnow receives a forcedstrtype variable.mmengine/structures/instance_data.py, static annotation is modified.Argument 2 to "copy" has incompatible type "str | Path"; expected "str" [arg-type]
mmengine/fileio/backends/local_backend.py,shutil.copy(src, dst)is modified toshutil.copy(str(src), str(dst))mmengine/fileio/backends/local_backend.py,shutil.copytree(src, dst)is modified toshutil.copytree(str(src), str(dst))Fix mmengine/runner/runner.py:1428: error: "Any" not callable [misc].
Fix mmengine/runner/runner.py:1439: error: Argument "num_workers" to "worker_init_fn" has incompatible type "Any | None"; expected "int" [arg-type].
dataloader_cfg.get('num_workers')is modified todataloader_cfg.get('num_workers', 0)The position of
# type: ignoreis modified to ensure it is effective.The similar issue also happens in
mmengine/runner/_flexible_runner.py, which is also modified.Accomplish TODO: if filepath or filepaths are Path, should return Path.
This TODO is described in
mmengine/fileio/backends/local_backend.py, this causes implementations and static annotations modification on func:join_path,copyfile,copytree,copyfile_from_local,copytree_from_local,copyfile_to_local,copytree_to_local.Now the fileio can dynamically operates
pathlib.Pathandstrinputs and will return different types.tests/test_fileio/test_backends/test_local_backend.pyis modified to align with the updated fileio API.test_join_pathis also influenced.Fix
TestEmptyCacheHookTestThe annotation in
TestEmptyCacheHooksuggestes it wants to test the times of calling oftorch.cuda.empty_cacheHook. The test logic hopesrunner.trainwill callmax_epochstimes. To achieve this, the test script specifiesval_intervalto1e6, but this cannot override the final validation on training last epoch, see theEpochBasedTrainLoopbelow:Merely specifying
val_intervalto1e6cannot prevent entering if whenself._epoch == self._max_epochs, i.e., when training done. To safely prevent entering this if block, setval_beginto infinite seems to be an option.BC-breaking
There shouldn't be any BC-breaking.