Skip to content
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

SDK: remove imports that were deprecated in Python 3.9 #8626

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

SpecLad
Copy link
Contributor

@SpecLad SpecLad commented Oct 31, 2024

Motivation and context

Since we just dropped support for Python 3.8, we don't need to keep using the old imports anymore.

This patch was produced by ruff check --select=UP006,UP035 --target-version=py39 --fix --unsafe-fixes (with a couple of manual fixes where ruff coudn't do it by itself).

I'd like to apply this across the entire codebase, but since it produces a large amount of churn, I'd rather do it piecemeal.
In particular, the SDK contains generated code with typing imports, but since I can't fix that automatically, I'm going to delay it until another PR.

How has this been tested?

I ran a case-insensitive diff to make sure that the only changes were imports, lowercasing of the type annotations, and replacing of ContextManager with AbstractContextManager.

Checklist

  • I submit my changes into the develop branch
  • [ ] 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

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.

Summary by CodeRabbit

Release Notes

  • New Features

    • No new features were introduced in this release.
  • Improvements

    • Updated type annotations throughout the codebase to use built-in types (list, dict, tuple, etc.) instead of their typing module counterparts, enhancing readability and consistency.
    • Refined import statements for collections, moving from typing to collections.abc.
  • Bug Fixes

    • No specific bug fixes were noted in this release.

These changes streamline the code and align with modern Python practices without altering existing functionality.

Copy link
Contributor

coderabbitai bot commented Oct 31, 2024

Walkthrough

The changes in this pull request primarily involve updates to type annotations across multiple files in the CVAT SDK, transitioning from types in the typing module (such as List, Dict, and Tuple) to their built-in equivalents (list, dict, and tuple). This shift reflects modern Python practices for type hinting. Additionally, some import statements have been modified to source types from collections.abc instead of typing. The core functionality and logic of the methods and classes remain unchanged.

Changes

File Path Change Summary
cvat-sdk/cvat_sdk/auto_annotation/driver.py Updated method signature for validate_and_remap to use list instead of List.
cvat-sdk/cvat_sdk/auto_annotation/functions/torchvision_detection.py Updated return type for detect method to list instead of List.
cvat-sdk/cvat_sdk/auto_annotation/functions/torchvision_keypoint_detection.py Updated return type for detect method to list instead of List.
cvat-sdk/cvat_sdk/auto_annotation/interface.py Updated return type for detect method to list instead of List and modified imports.
cvat-sdk/cvat_sdk/core/client.py Updated method signatures to use dict, tuple instead of Dict, Tuple.
cvat-sdk/cvat_sdk/core/downloading.py Updated parameters in methods to use dict instead of Dict.
cvat-sdk/cvat_sdk/core/helpers.py Updated return type for get_paginated_collection to use list instead of List.
cvat-sdk/cvat_sdk/core/progress.py Changed import statements for Generator and Iterable to collections.abc.
cvat-sdk/cvat_sdk/core/proxies/annotations.py Updated remove_annotations method to use Optional[Sequence[int]].
cvat-sdk/cvat_sdk/core/proxies/issues.py Updated return type for get_comments method to list instead of List.
cvat-sdk/cvat_sdk/core/proxies/jobs.py Updated return types for multiple methods to use list instead of List.
cvat-sdk/cvat_sdk/core/proxies/model_proxy.py Updated various method signatures to use dict, list, and tuple instead of Dict, List, Tuple.
cvat-sdk/cvat_sdk/core/proxies/projects.py Updated return types for get_tasks and get_labels methods to list instead of List.
cvat-sdk/cvat_sdk/core/proxies/tasks.py Updated return types for multiple methods to use list instead of List.
cvat-sdk/cvat_sdk/core/uploading.py Updated parameters and return types in various methods to use dict and list.
cvat-sdk/cvat_sdk/core/utils.py Updated filter_dict function to use dict instead of Dict and adjusted context manager types.
cvat-sdk/cvat_sdk/datasets/caching.py Updated _CacheObject type alias to dict and various method signatures to use type.
cvat-sdk/cvat_sdk/datasets/common.py Updated attribute types in FrameAnnotations class to use list instead of List.
cvat-sdk/cvat_sdk/datasets/task_dataset.py Updated import statements to use collections.abc.
cvat-sdk/cvat_sdk/pytorch/common.py Changed import of Mapping from typing to collections.abc.
cvat-sdk/cvat_sdk/pytorch/project_dataset.py Changed import statements for Container and Mapping to collections.abc.
cvat-sdk/cvat_sdk/pytorch/task_dataset.py Updated import for Mapping and adjusted type hint for label_name_to_index.
cvat-sdk/cvat_sdk/pytorch/transforms.py Updated include_shape_types attribute type to frozenset instead of FrozenSet.

Poem

In the land of code where rabbits play,
Types were updated in a modern way.
From List to list, oh what a delight,
With built-in types, everything feels right!
Hopping through changes, our code is now bright,
A leap into clarity, oh what a sight! 🐇✨


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (1)
cvat-sdk/cvat_sdk/core/client.py (1)

Type hint updates needed across multiple files

Several files still use deprecated type hints from the typing module that should be updated:

  • Replace List, Dict, Tuple, Set with their built-in lowercase equivalents
  • Move Generator and Sequence imports to use collections.abc instead of typing

Key files requiring updates:

  • utils/dataset_manifest/core.py
  • cvat-cli/src/cvat_cli/cli.py
  • tests/python/rest_api/utils.py
  • cvat/apps/engine/views.py
  • cvat/apps/dataset_manager/bindings.py
  • And 40+ other files shown in the scan results
🔗 Analysis chain

Line range hint 10-335: Verify completeness of type hint updates.

Let's verify that all deprecated type hints from the typing module have been updated to their built-in equivalents across the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining deprecated type hints from typing module
# that should be using built-in types or collections.abc

# Test: Search for capital-case type hints that should be lowercase
rg -g '*.py' 'from typing import .*(?:List|Dict|Tuple|Set)' || echo "No deprecated List/Dict/Tuple/Set imports found"

# Test: Search for direct usage of capital-case type hints
rg -g '*.py' '(?<!from typing import )(?:List|Dict|Tuple|Set)\[' || echo "No direct usage of List/Dict/Tuple/Set found"

# Test: Search for types that should be from collections.abc
rg -g '*.py' 'from typing import .*(?:Generator|Sequence)' || echo "No deprecated Generator/Sequence imports found"

Length of output: 6709

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between a652191 and be1d636.

📒 Files selected for processing (23)
  • cvat-sdk/cvat_sdk/auto_annotation/driver.py (2 hunks)
  • cvat-sdk/cvat_sdk/auto_annotation/functions/torchvision_detection.py (1 hunks)
  • cvat-sdk/cvat_sdk/auto_annotation/functions/torchvision_keypoint_detection.py (1 hunks)
  • cvat-sdk/cvat_sdk/auto_annotation/interface.py (2 hunks)
  • cvat-sdk/cvat_sdk/core/client.py (6 hunks)
  • cvat-sdk/cvat_sdk/core/downloading.py (3 hunks)
  • cvat-sdk/cvat_sdk/core/helpers.py (2 hunks)
  • cvat-sdk/cvat_sdk/core/progress.py (1 hunks)
  • cvat-sdk/cvat_sdk/core/proxies/annotations.py (1 hunks)
  • cvat-sdk/cvat_sdk/core/proxies/issues.py (1 hunks)
  • cvat-sdk/cvat_sdk/core/proxies/jobs.py (4 hunks)
  • cvat-sdk/cvat_sdk/core/proxies/model_proxy.py (5 hunks)
  • cvat-sdk/cvat_sdk/core/proxies/projects.py (2 hunks)
  • cvat-sdk/cvat_sdk/core/proxies/tasks.py (6 hunks)
  • cvat-sdk/cvat_sdk/core/uploading.py (8 hunks)
  • cvat-sdk/cvat_sdk/core/utils.py (1 hunks)
  • cvat-sdk/cvat_sdk/datasets/caching.py (9 hunks)
  • cvat-sdk/cvat_sdk/datasets/common.py (2 hunks)
  • cvat-sdk/cvat_sdk/datasets/task_dataset.py (1 hunks)
  • cvat-sdk/cvat_sdk/pytorch/common.py (1 hunks)
  • cvat-sdk/cvat_sdk/pytorch/project_dataset.py (1 hunks)
  • cvat-sdk/cvat_sdk/pytorch/task_dataset.py (1 hunks)
  • cvat-sdk/cvat_sdk/pytorch/transforms.py (2 hunks)
✅ Files skipped from review due to trivial changes (3)
  • cvat-sdk/cvat_sdk/core/progress.py
  • cvat-sdk/cvat_sdk/datasets/task_dataset.py
  • cvat-sdk/cvat_sdk/pytorch/common.py
🔇 Additional comments (61)
cvat-sdk/cvat_sdk/auto_annotation/functions/torchvision_detection.py (1)

30-30: LGTM! Type annotation change follows PEP 585

The change from List to list aligns with Python 3.9+ type hinting best practices and matches the actual return type of the method.

cvat-sdk/cvat_sdk/datasets/common.py (1)

27-28: LGTM! Verify minimum Python version requirement.

The changes from List to list type annotations are correct and align with modern Python practices. However, since built-in type annotations require Python 3.9+, we should verify the project's minimum Python version requirement.

✅ Verification successful

Let me analyze the code changes and the review comment to verify the Python version compatibility concern.

The script will help me check the Python version requirements specified in the project configuration files.


Let me analyze the findings and provide a final response.

The change from List to list type annotations is safe.

The code changes involve switching from typing.List to built-in list type annotations. This is safe because:

  1. The project's dependencies show Python 3.8+ compatibility (e.g. Django 4.2.x requires Python 3.8+)
  2. Built-in type annotations for list were introduced in Python 3.7+ (PEP 585)
  3. The change is part of modernizing the codebase and follows Python's recommendations for using built-in types for annotations where possible

The change aligns with Python's type annotation evolution and maintains backward compatibility with the project's minimum Python version requirements.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check minimum Python version requirement in project metadata

# Look for Python version specifications in pyproject.toml or setup.py
fd -e toml -e py -x grep -l "python" {} \; | xargs cat

Length of output: 564347


Script:

#!/bin/bash
# Check Python version requirements in project metadata files
fd -e toml -e txt -x grep -l "python" {} \; | xargs cat

Length of output: 9160

cvat-sdk/cvat_sdk/core/proxies/issues.py (1)

54-54: LGTM! Type annotation updated correctly.

The change from List[Comment] to list[Comment] aligns with PEP 585 and modern Python type hinting practices.

Let's verify the consistency of type annotations and imports:

✅ Verification successful

Type annotation change is consistent and correct

Looking at the complete file content:

  1. The file already uses from __future__ import annotations
  2. No typing.List or from typing import List imports are present
  3. The return type annotation list[Comment] is the only collection type annotation in the file
  4. All other type hints use proper PEP 585 style with generics
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining typing.List imports and usage
# Also check for consistency in type annotation style across the file

# Test 1: Check for any remaining 'typing.List' or 'from typing import List'
rg -l "(?:from typing import .*List|typing\.List)" cvat-sdk/cvat_sdk/core/proxies/issues.py

# Test 2: Look for any capital-case List type annotations that might have been missed
rg -l "\bList\[" cvat-sdk/cvat_sdk/core/proxies/issues.py

# Expected: No output from either command, indicating all List references have been updated

Length of output: 153


Script:

#!/bin/bash
# Let's check the entire file content to verify type annotations and imports
cat cvat-sdk/cvat_sdk/core/proxies/issues.py

# Also check for any other type annotations in the file
rg "->.*\[" cvat-sdk/cvat_sdk/core/proxies/issues.py

Length of output: 2218

cvat-sdk/cvat_sdk/auto_annotation/functions/torchvision_keypoint_detection.py (1)

38-38: LGTM! Type annotation updated correctly.

The change from List to list aligns with PEP 585 and modern Python type hinting practices, correctly implementing the deprecation fix.

Let's verify the consistency of similar changes across the codebase:

✅ Verification successful

Type annotation changes are consistent across the SDK

The verification shows that the codebase is consistent with the PEP 585 changes. The only remaining typing.List usage is in gen/postprocess.py, which is a code generation utility that deliberately adds typing imports during code generation. This is expected and correct behavior as it's part of the code generation process, not actual type annotations in the SDK code.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining usage of typing.List in the SDK
# Expect: No occurrences of 'from typing import List' or 'typing.List'

echo "Checking for remaining 'typing.List' usage..."
rg -g 'cvat-sdk/**/*.py' 'typing\.List'

echo "Checking for remaining 'from typing import List' statements..."
rg -g 'cvat-sdk/**/*.py' 'from\s+typing\s+import.*List'

Length of output: 434


Script:

#!/bin/bash
# Let's check the context of the remaining typing.List usage in gen/postprocess.py
# and also search for any List[] type annotations in the codebase

echo "Checking context of typing.List usage in postprocess.py..."
rg -B 3 -A 3 'typing\.List' cvat-sdk/gen/postprocess.py

echo -e "\nChecking for List[] type annotations in SDK..."
rg -g 'cvat-sdk/**/*.py' '\bList\['

Length of output: 743

cvat-sdk/cvat_sdk/core/utils.py (3)

10-11: LGTM: Proper migration to collections.abc

The change correctly moves collection types to collections.abc, following Python 3.9+ recommendations.


15-16: LGTM: Proper migration to built-in type annotations

The change correctly updates the type hints from Dict to dict, following modern Python type hinting practices.


23-23: LGTM: Enhanced type precision with AbstractContextManager

The change from ContextManager to AbstractContextManager is an improvement as it more precisely specifies the contract that the context manager implements. This provides better type safety and clearer interface documentation.

Also applies to: 29-29

cvat-sdk/cvat_sdk/core/proxies/annotations.py (3)

6-6: LGTM: Import change aligns with Python 3.9+ recommendations

The change to import Sequence from collections.abc instead of typing follows the recommended practice for Python 3.9 and later versions.


Line range hint 47-47: LGTM: Type hint accurately reflects parameter usage

The type hint Optional[Sequence[int]] for the ids parameter is appropriate as:

  1. It correctly indicates that the parameter can be None
  2. Using Sequence instead of a more specific type like List is good practice as the implementation only requires iteration capability

6-8: Verify other typing imports

The file still imports Optional from typing. Consider checking if other typing imports could be moved to their modern locations.

cvat-sdk/cvat_sdk/pytorch/transforms.py (2)

5-5: LGTM! Import usage is correct.

The TypedDict import from typing is appropriate as it wasn't deprecated in Python 3.9.


Line range hint 66-70: Type annotation change looks good.

The update from FrozenSet[str] to frozenset[str] aligns with PEP 585 and the PR's objective to modernize type hints. The field's converter and validator remain properly configured.

Let's verify no deprecated typing imports remain in the file:

✅ Verification successful

Type annotation change is correctly implemented

The file only imports TypedDict from typing module and uses built-in frozenset type annotation, which is correct according to PEP 585. No deprecated typing imports remain in the file, and the type annotation change is properly integrated with the attrs field definition.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining deprecated typing imports
# Expected: No matches for deprecated types like List, Dict, Set, etc.

rg -i "from typing import (?:.+,\s*)?(List|Dict|Set|FrozenSet|Tuple|Type|Callable)[,\s]" "cvat-sdk/cvat_sdk/pytorch/transforms.py"

Length of output: 130


Script:

#!/bin/bash
# Let's check all typing-related imports in the file
rg "^from typing import" -A 1 "cvat-sdk/cvat_sdk/pytorch/transforms.py"

# Also check the entire file content to understand the context better
cat "cvat-sdk/cvat_sdk/pytorch/transforms.py"

Length of output: 3028

cvat-sdk/cvat_sdk/core/helpers.py (2)

10-11: LGTM! Correct migration of deprecated imports

The change properly moves Iterable from typing to collections.abc, following PEP 585 recommendations for Python 3.9+. The remaining imports from typing are correctly maintained as they aren't collection types.


23-23: LGTM! Proper update of type annotations

The return type annotation has been correctly updated to use built-in generic types (list, dict) instead of their typing module equivalents (List, Dict), following PEP 585 guidelines for Python 3.9+.

cvat-sdk/cvat_sdk/core/downloading.py (3)

83-84: LGTM! Type annotation updates align with PEP 585.

The change from Dict to dict follows modern Python practices by using built-in collection types as generic types, which was introduced in Python 3.9.


121-122: LGTM! Consistent type annotation updates.

The type hints are consistently updated to use built-in dict type, maintaining uniformity across the codebase.


11-11: Verify remaining typing imports.

While the changes look good, there might be other opportunities to modernize type hints in this file. Let's verify if there are any remaining uses of deprecated typing imports that could be updated.

cvat-sdk/cvat_sdk/pytorch/task_dataset.py (2)

Line range hint 41-42: LGTM! Type hint usage is correct.

The Mapping[str, int] type hint is appropriately used for the label_name_to_index parameter, maintaining read-only mapping semantics and correctly handling the optional nature of the parameter.


7-8: LGTM! Import changes align with PEP 585.

The change to import Mapping from collections.abc instead of typing is correct and follows Python 3.9+ recommendations.

Let's verify if there are other typing imports that could be migrated:

✅ Verification successful

Import changes are correct and no other migrations needed

After reviewing the complete file, the only collection type imported from typing is Optional and Callable, which are not collection types and should remain imported from typing. The migration of Mapping to collections.abc is correct, and there are no other collection-related imports from typing that need to be migrated.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other typing imports that could potentially be migrated
# to their built-in equivalents or collections.abc

# Look for remaining typing imports that might be candidates for migration
rg "from typing import .*?(List|Dict|Set|FrozenSet|Tuple|Type|Mapping|MutableMapping|Sequence|MutableSequence|AbstractSet|MutableSet|ByteString|Deque|DefaultDict|OrderedDict|Counter|ChainMap|AsyncGenerator|AsyncIterator|AsyncIterable|Awaitable|Coroutine|Collection|Container|ItemsView|KeysView|ValuesView|Reversible|Generator|Iterator|Iterable)" "cvat-sdk/cvat_sdk/pytorch/task_dataset.py"

Length of output: 389


Script:

#!/bin/bash
# Let's first check the entire content of the file to understand all imports and their usage
cat "cvat-sdk/cvat_sdk/pytorch/task_dataset.py"

Length of output: 4410

cvat-sdk/cvat_sdk/core/proxies/jobs.py (3)

9-11: LGTM! Import changes align with PEP 585.

The migration of Sequence from typing to collections.abc follows the deprecation guidelines while correctly maintaining other necessary imports from typing.


97-97: LGTM! Type annotations updated to use built-in collection types.

The changes consistently replace List with list across all method signatures, following PEP 585 guidelines. The modifications maintain the same type semantics while using the more modern syntax.

Also applies to: 129-129, 134-134, 145-145


9-11: Verify remaining typing imports in SDK.

Let's ensure no other deprecated typing imports remain in the SDK directory.

✅ Verification successful

No deprecated typing imports found in SDK

The verification shows that:

  • All sequence-like type hints are properly imported from collections.abc
  • No deprecated imports from typing module were found, except for TypedDict which is correct as it's not part of collections.abc
  • The codebase shows consistent usage of collections.abc imports across all SDK files
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining imports from typing that should be from collections.abc
# Focus on commonly deprecated types as per PEP 585

rg -g 'cvat-sdk/**/*.py' 'from typing import.*(Sequence|List|Dict|Set|Tuple|FrozenSet|Deque|DefaultDict|OrderedDict|Counter|ChainMap)'

Length of output: 205


Script:

#!/bin/bash
# Let's verify if there are any direct imports of these types without 'from typing import'
rg -g 'cvat-sdk/**/*.py' '^import typing.*'

# Also check for any remaining collections.abc imports to ensure consistency
rg -g 'cvat-sdk/**/*.py' 'from collections.abc import'

Length of output: 1233

cvat-sdk/cvat_sdk/core/proxies/projects.py (3)

10-10: LGTM! Import statement correctly updated.

The import statement has been properly updated to only import necessary types (Optional, TYPE_CHECKING) from typing module, aligning with Python 3.9+ recommendations to use built-in types instead of those from typing.


75-80: LGTM! Return type annotation correctly modernized.

The return type annotation has been properly updated from List[Task] to list[Task], following PEP 585 guidelines for using built-in collection types in Python 3.9+. The implementation correctly matches the return type.


83-86: LGTM! Return type annotation correctly modernized.

The return type annotation has been properly updated from List[models.ILabel] to list[models.ILabel], following PEP 585 guidelines. The implementation using get_paginated_collection correctly matches the return type.

cvat-sdk/cvat_sdk/auto_annotation/interface.py (2)

6-6: LGTM! Import change follows Python 3.9+ recommendations.

The change from typing to collections.abc for Sequence aligns with modern Python practices and removes usage of deprecated imports.


83-83: LGTM! Return type annotation updated to use built-in generic type.

The change from List to list follows PEP 585 recommendations for Python 3.9+.

Let's verify that similar changes have been made consistently across the codebase:

✅ Verification successful

Return type annotation change is consistent with codebase practices

The change from List to list is appropriate. The only remaining usage of List is in:

  • postprocess.py which is a code generator that explicitly needs to generate typing.List imports
  • cvat/apps/engine/task.py which is outside the SDK and not in scope for this change
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining usage of typing.List in the SDK
# We should not find any instances of typing.List after this change

echo "Checking for remaining 'typing.List' or 'from typing import List' usage in SDK..."
rg -t py "(?:typing\.List|from typing import .*\bList\b.*)" cvat-sdk/

echo "Checking for any List[] type annotations that might need updating..."
rg -t py "\bList\[" cvat-sdk/

Length of output: 609


Script:

#!/bin/bash
# Let's check the context of the postprocess.py usage to understand if it's a special case
echo "Checking the context of List usage in postprocess.py..."
rg -A 5 -B 5 "type_repr.*List" cvat-sdk/gen/postprocess.py

# Also verify if there are any actual List type annotations in use
echo "Checking for List type annotations in actual code..."
ast-grep --pattern 'List[$$$]'

Length of output: 902

cvat-sdk/cvat_sdk/auto_annotation/driver.py (2)

123-123: LGTM! Type annotation updated correctly.

The parameter type annotation change from List[models.LabeledShapeRequest] to list[models.LabeledShapeRequest] is correct and consistent with the move away from typing.List.


6-7: LGTM! Import changes align with PEP 585.

The change from typing to collections.abc for Mapping and Sequence is correct and follows the deprecation guidelines in Python 3.9+.

Let's verify if there are any remaining deprecated typing imports that should be updated:

✅ Verification successful

Import changes are correctly implemented

The verification confirms that:

  1. Only Optional from typing module remains, which is correct as it's not deprecated
  2. Mapping and Sequence are correctly imported from collections.abc
  3. No other deprecated typing imports are present in the file
  4. No direct typing.* references are found in the code
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining deprecated typing imports
rg "from typing import (?:.*, )?(List|Dict|Set|Tuple|FrozenSet|Type|Deque|DefaultDict|OrderedDict|Counter|ChainMap)" "cvat-sdk/cvat_sdk/auto_annotation/driver.py"

Length of output: 162


Script:

#!/bin/bash
# Let's check all imports from typing module to ensure completeness
rg "^from typing import" "cvat-sdk/cvat_sdk/auto_annotation/driver.py" -A 1

# Also check for direct typing usage in type annotations
rg "typing\." "cvat-sdk/cvat_sdk/auto_annotation/driver.py"

# Let's see the content of the file to verify the context
cat "cvat-sdk/cvat_sdk/auto_annotation/driver.py"

Length of output: 11404

cvat-sdk/cvat_sdk/datasets/caching.py (7)

9-9: LGTM! Import changes align with PEP 585.

The changes correctly move the Mapping import from typing to collections.abc, following Python 3.9's deprecation of these imports from the typing module.

Also applies to: 12-12


43-43: LGTM! Type alias updated to use built-in type.

The change from Dict[str, Any] to dict[str, Any] follows modern Python type hinting practices while maintaining the same functionality.


110-110: LGTM! Method signatures updated to use built-in type.

The changes from Type[_ModelType] to type[_ModelType] in method signatures follow modern Python type hinting practices.

Also applies to: 124-124


229-229: LGTM! Method signature updated consistently.

The change from Type[_ModelType] to type[_ModelType] maintains consistency with other similar changes in the file.


251-251: LGTM! Type annotations updated to use built-in list type.

The changes from List[models.ILabel] to list[models.ILabel] follow modern Python type hinting practices.

Also applies to: 289-290


282-282: LGTM! Method return types updated consistently.

The changes from List[...] to list[...] in method return type annotations maintain consistency with modern Python type hinting practices.

Also applies to: 324-324, 327-327


331-331: LGTM! Type annotation updated consistently.

The change from Type[CacheManager] to type[CacheManager] in the cache manager mapping maintains consistency with modern Python type hinting practices.

cvat-sdk/cvat_sdk/core/client.py (5)

10-14: LGTM: Correct migration of type imports.

The change correctly moves Generator and Sequence imports from typing to collections.abc, following PEP 585 recommendations for Python 3.9+.


99-99: LGTM: Correct update to built-in type annotation.

The type hint for _repos is correctly updated to use the built-in dict type instead of Dict from typing module.


187-187: LGTM: Correct update to built-in type annotation.

The parameter type hint for credentials is correctly updated to use the built-in tuple type instead of Tuple from typing module.


215-215: LGTM: Correct update to built-in type annotation.

The return type hint is correctly updated to use the built-in tuple type instead of Tuple from typing module.


323-324: LGTM: Correct update to built-in type annotations.

The parameter type hints are correctly updated to use built-in types instead of their typing module counterparts:

  • kwsub and query_params: dict[str, Any]
  • credentials: Optional[tuple[str, str]]

Also applies to: 335-335

cvat-sdk/cvat_sdk/core/proxies/model_proxy.py (6)

95-95: LGTM: Type hint modernization

The change from Type to type aligns with PEP 585 and modern Python type hinting practices.


102-103: LGTM: Consistent type hint updates

The parameter and return type annotations have been correctly updated to use built-in types, maintaining type safety while following PEP 585 guidelines.


127-127: LGTM: Updated type hint in create method

The type hint change from Dict to dict follows PEP 585 guidelines while maintaining the same type safety.


148-153: LGTM: Updated list method overload type hints

The type hints in all overloads have been correctly updated to use built-in list type, following PEP 585 guidelines while maintaining type safety.


173-174: LGTM: Updated type hints in update-related methods

The type hints in _export_update_fields and update methods have been correctly updated to use built-in dict type, following PEP 585 guidelines while maintaining type safety.

Also applies to: 193-193


Line range hint 1-353: Verify removal of deprecated typing imports

Let's verify that no deprecated typing imports remain in the file.

✅ Verification successful

No deprecated typing imports found, changes are correct

The verification confirms that:

  • No deprecated Dict, List, or Tuple imports from typing module
  • All collection types are using built-in generics (e.g., list[_EntityT], dict[str, Any], tuple[...])
  • Type hints follow modern Python typing practices (PEP 585)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining deprecated typing imports
# Note: These patterns were deprecated in Python 3.9

# Test: Search for deprecated typing imports
echo "Checking for deprecated typing imports..."
rg -U 'from typing import .*(?:Dict|List|Tuple)' cvat-sdk/cvat_sdk/core/proxies/model_proxy.py

# Test: Search for usage of deprecated types
echo "Checking for usage of deprecated types..."
rg '\b(Dict|List|Tuple)\[' cvat-sdk/cvat_sdk/core/proxies/model_proxy.py

Length of output: 357


Script:

#!/bin/bash
# Let's verify more thoroughly with different patterns and include type annotations

# Check for any imports from typing module
echo "All typing imports:"
rg "^from typing import" cvat-sdk/cvat_sdk/core/proxies/model_proxy.py

# Check all type annotations in the file
echo -e "\nAll type annotations:"
ast-grep --pattern 'annotation: $_'

# Check specifically for collection type usage
echo -e "\nUsage of collection types:"
rg -A 1 '(?:list|dict|tuple)\[' cvat-sdk/cvat_sdk/core/proxies/model_proxy.py

Length of output: 1942

cvat-sdk/cvat_sdk/core/proxies/tasks.py (8)

11-11: LGTM: Correct import source for Sequence

The change to import Sequence from collections.abc instead of typing aligns with PEP 585 and modern Python practices.


76-76: LGTM: Correct type annotation update for params parameter

The change from Dict to dict aligns with PEP 585 and modern type hinting practices.


230-230: LGTM: Correct type annotation update for return type

The change from List to list aligns with PEP 585 and modern type hinting practices.


257-257: LGTM: Correct type annotation update for return type

The change from List to list aligns with PEP 585 and modern type hinting practices.


269-269: LGTM: Correct type annotation update for return type

The change from List to list aligns with PEP 585 and modern type hinting practices.


274-274: LGTM: Correct type annotation update for return type

The change from List to list aligns with PEP 585 and modern type hinting practices.


299-299: LGTM: Correct type annotation update for data_params parameter

The change from Dict to dict aligns with PEP 585 and modern type hinting practices.


Line range hint 11-299: Verify remaining typing imports in the file

All type annotation changes look good and are consistent. Let's verify if there are any remaining deprecated typing imports that should be updated.

✅ Verification successful

All type annotations are properly modernized

The file only imports TYPE_CHECKING, Any, and Optional from the typing module, which are still required as they don't have built-in equivalents. All other type annotations use modern syntax with built-in collection types (dict, Sequence) and no deprecated typing imports (List, Dict, Tuple, etc.) are present.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining deprecated typing imports that could be updated
# Test: Look for type annotations from typing module that could be replaced with built-in types
rg -U 'from typing import .*(?:List|Dict|Tuple|Set|FrozenSet)' cvat-sdk/cvat_sdk/core/proxies/tasks.py

# Test: Look for direct usage of typing.List/Dict/etc in type annotations
rg -U '(?:List|Dict|Tuple|Set|FrozenSet)\[' cvat-sdk/cvat_sdk/core/proxies/tasks.py

Length of output: 190


Script:

#!/bin/bash
# Let's do a more comprehensive check of typing imports and their usage
# First, let's see all imports from typing module
rg "^from typing import" cvat-sdk/cvat_sdk/core/proxies/tasks.py

# Then check all type annotations in the file
rg ":[^=]+" cvat-sdk/cvat_sdk/core/proxies/tasks.py

# Also check for any direct usage of typing module
rg "typing\." cvat-sdk/cvat_sdk/core/proxies/tasks.py

Length of output: 2320

cvat-sdk/cvat_sdk/core/uploading.py (6)

9-11: LGTM: Appropriate replacement of deprecated import

The change from typing.ContextManager to contextlib.AbstractContextManager is correct and aligns with Python's deprecation notices. This is the recommended way to import context manager types in modern Python.


151-153: LGTM: Proper modernization of type hints

The transition from Dict to dict in type annotations follows modern Python type hinting best practices. These built-in types are now preferred over their typing module counterparts.


198-198: LGTM: Consistent type annotation update

The return type annotation change to AbstractContextManager[None] is consistent with the import changes and correctly specifies that the context manager doesn't return a value.


260-260: LGTM: Consistent type hint modernization

The type annotation update for url_params maintains consistency with the modern type hinting approach used throughout the file.


306-307: LGTM: Appropriate type hint update

The change from List[Path] to list[Path] follows the modern Python type hinting conventions while maintaining the correct type specification for the resources parameter.


Line range hint 355-375: LGTM: Comprehensive type hint modernization

The type annotation updates in the _split_files_by_requests method are thorough and consistent, properly converting all typing module types to their built-in equivalents while maintaining the correct type structure for complex nested types.

cvat-sdk/cvat_sdk/pytorch/project_dataset.py Show resolved Hide resolved
@SpecLad SpecLad merged commit 5dc5265 into cvat-ai:develop Nov 1, 2024
34 checks passed
@SpecLad SpecLad deleted the deprecated-typing branch November 1, 2024 15:29
SpecLad added a commit to SpecLad/cvat that referenced this pull request Nov 1, 2024
SpecLad added a commit that referenced this pull request Nov 4, 2024
SpecLad added a commit that referenced this pull request Nov 5, 2024
SpecLad added a commit that referenced this pull request Nov 6, 2024
And also `abstractstaticmethod`, which was deprecated in 3.3.

This is a continuation of #8626.
SpecLad added a commit that referenced this pull request Nov 11, 2024
SpecLad added a commit that referenced this pull request Nov 14, 2024
SpecLad added a commit to SpecLad/cvat that referenced this pull request Nov 26, 2024
SpecLad added a commit to SpecLad/cvat that referenced this pull request Nov 26, 2024
SpecLad added a commit that referenced this pull request Nov 26, 2024
SpecLad added a commit to SpecLad/cvat that referenced this pull request Dec 20, 2024
@SpecLad SpecLad mentioned this pull request Dec 20, 2024
2 tasks
SpecLad added a commit that referenced this pull request Dec 20, 2024
SpecLad added a commit to SpecLad/cvat that referenced this pull request Dec 23, 2024
This concludes the series started in cvat-ai#8626.
SpecLad added a commit that referenced this pull request Dec 24, 2024
This concludes the series started in #8626.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant