CLI: remove imports that were deprecated in Python 3.9 #8640
Conversation
WalkthroughThe changes in this pull request involve updating type hints across multiple files in the CVAT CLI codebase. The modifications primarily transition from using types from the 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: 0
🧹 Outside diff range and nitpick comments (1)
cvat-cli/src/cvat_cli/_internal/commands.py (1)
473-475: LGTM: Type hint modernization with a suggestion.The change from
Dict[str, Any]todict[str, Any]aligns with PEP 585. Given the dynamic nature offunction_parameters, consider adding a docstring to document expected parameter types and formats.Consider adding a docstring like:
def execute( self, client: Client, *, task_id: int, function_parameters: dict[str, Any], clear_existing: bool = False, allow_unmatched_labels: bool = False, ) -> None: """Execute auto-annotation on a task. Args: client: CVAT client instance task_id: ID of the task to annotate function_parameters: Dictionary of parameters passed to the annotation function. The values can be of any type supported by the function. clear_existing: Whether to remove existing annotations allow_unmatched_labels: Whether to allow labels not in task configuration """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
cvat-cli/src/cvat_cli/__main__.py(1 hunks)cvat-cli/src/cvat_cli/_internal/command_base.py(2 hunks)cvat-cli/src/cvat_cli/_internal/commands.py(3 hunks)cvat-cli/src/cvat_cli/_internal/parsers.py(2 hunks)
🔇 Additional comments (7)
cvat-cli/src/cvat_cli/__main__.py (1)
20-20: LGTM! Type hint modernization looks good.
The change from List[str] to list[str] aligns with PEP 585 and improves Python 3.9+ compatibility.
Let's verify if there are any remaining uses of deprecated type hints that need similar updates:
✅ Verification successful
Based on the search results, there are still many occurrences of deprecated type hints from the typing module (List, Dict, Tuple) across the codebase. However, since this PR is focused on modernizing type hints in the CLI component, and the change in __main__.py is consistent with that scope, I'll generate the final response:
Type hint modernization is correctly scoped to CLI component
The change from List[str] to list[str] in __main__.py is appropriate and aligns with PEP 585. While there are other files in the codebase still using the deprecated typing module types, those changes should be handled in separate PRs to maintain focused scope and minimize risk.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for remaining usage of typing.List, typing.Dict, typing.Tuple
# that might need to be updated to use built-in types instead
echo "Checking for remaining deprecated type hints..."
rg -l "from typing import .*(?:List|Dict|Tuple)" || true
rg "\b(?:List|Dict|Tuple)\[" || true
Length of output: 31111
cvat-cli/src/cvat_cli/_internal/parsers.py (2)
9-9: LGTM! Correctly retained Any import.
The code correctly maintains the import of Any from typing, which is necessary as it has no built-in equivalent, while removing deprecated imports.
31-31: LGTM! Appropriate use of built-in generic type.
The change from Tuple[str, Any] to tuple[str, Any] aligns with PEP 585, which enables the use of built-in collections as generic types in Python 3.9+.
Let's verify the Python version requirement is properly documented:
✅ Verification successful
Let me gather more information by checking other potential locations where Python version requirements might be documented.
Python version requirement is properly documented
The Python version requirement (>=3.9) is correctly documented in the setup.py file:
python_requires=">=3.9"This aligns with the usage of built-in generic types like tuple[str, Any] which were introduced in Python 3.9. Additionally, the black configuration in pyproject.toml also targets Python 3.9 with target-version = ['py39'].
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if Python version requirement is documented
# Expected: Should find Python version requirement >= 3.9 in setup files or requirements
rg -l "python.*3\.9" --type yaml --type toml
cat pyproject.toml 2>/dev/null || true
cat setup.cfg 2>/dev/null || true
Length of output: 945
Script:
#!/bin/bash
# Check setup.py, requirements files, and README for Python version requirements
fd -e py setup.py -x cat {}
fd -e txt -e in requirements -x cat {}
cat README.md 2>/dev/null || true
Length of output: 20873
cvat-cli/src/cvat_cli/_internal/command_base.py (2)
7-7: LGTM! Correct migration of Mapping import.
The change to import Mapping from collections.abc instead of typing aligns with PEP 585 and Python 3.9+ best practices.
25-25: LGTM! Appropriate modernization of type hints.
The change from Dict[str, Command] to dict[str, Command] follows PEP 585, which enables the use of built-in collections as generic types. This is part of the broader effort to modernize type hints across the codebase while maintaining type safety.
cvat-cli/src/cvat_cli/_internal/commands.py (2)
12-12: LGTM: Import changes align with PEP 585.
The changes correctly migrate from deprecated typing collection types to their built-in counterparts, following Python 3.9+ best practices.
Also applies to: 14-14
207-207: LGTM: Type hint update follows modern Python practices.
The change from List[Dict[str, str]] to list[dict[str, str]] aligns with PEP 585.
Let's verify backward compatibility with older Python versions:
✅ Verification successful
Type hint change is safe and compatible with the codebase requirements
The verification confirms this change is safe because:
- The file already uses
from __future__ import annotations, which provides forward compatibility - The package requires Python >= 3.9 (specified in setup.py), which fully supports PEP 585 built-in collection type hints
- No conditional typing imports or version-specific type handling is present in the file
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any version-specific imports or type checking
# that might break with this change.
# Look for any Python version checks or typing-related conditional imports
rg -l "sys.version_info|typing.TYPE_CHECKING|from __future__ import annotations"
# Look for any typing-related configuration in pyproject.toml or setup.cfg
fd -e toml -e cfg -x cat {} | rg "^python_requires|typing"
Length of output: 1282
Script:
#!/bin/bash
# Let's check the specific file for any Python version requirements or typing imports
rg "from typing|from __future__|import typing|python_version|python_requires" cvat-cli/src/cvat_cli/_internal/commands.py -A 2
# Check for Python version requirements in the package configuration
fd "setup.(py|cfg)" "cvat-cli" -x cat {}
# Check for pyproject.toml in cvat-cli directory
fd "pyproject.toml" "cvat-cli" -x cat {}
Length of output: 2535
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #8640 +/- ##
===========================================
- Coverage 74.30% 74.26% -0.04%
===========================================
Files 401 401
Lines 43419 43421 +2
Branches 3951 3951
===========================================
- Hits 32263 32248 -15
- Misses 11156 11173 +17
|
Motivation and context
This is a continuation of #8626.
How has this been tested?
Checklist
developbranch[ ] I have created a changelog fragment[ ] I have updated the documentation accordingly[ ] I have added tests to cover my changes[ ] I have linked related issues (see GitHub docs)[ ] I have increased versions of npm packages if it is necessary(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
ListandDictto built-inlistanddictfor various methods and attributes across multiple files.Documentation