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

engine: remove deprecated imports #8856

Merged
merged 1 commit into from
Dec 20, 2024
Merged

Conversation

SpecLad
Copy link
Contributor

@SpecLad SpecLad commented Dec 20, 2024

Motivation and context

This is a continuation of #8626.

How has this been tested?

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

    • Introduced new fields and models in the migration for enhanced data handling.
  • Improvements

    • Updated type annotations across various files to use built-in types for better readability and consistency.
    • Enhanced error handling in cloud storage validation methods.
  • Bug Fixes

    • Streamlined job status handling during export processes.
  • Documentation

    • Improved clarity in method signatures and type hints throughout the codebase.
  • Chores

    • Refined import statements for better organization and adherence to modern Python practices.

@SpecLad SpecLad requested a review from Marishka17 as a code owner December 20, 2024 16:27
Copy link
Contributor

coderabbitai bot commented Dec 20, 2024

Walkthrough

The pull request introduces comprehensive type annotation updates across multiple files in the CVAT engine module. The changes primarily focus on modernizing type hints by using built-in Python types like dict, list, and tuple instead of their typing module counterparts. Additionally, the project is standardizing imports from collections.abc for collection-related types, enhancing type clarity and adherence to contemporary Python typing conventions. These modifications do not alter the core functionality of the existing code but improve type hinting consistency and readability.

Changes

File Change Summary
background.py Updated type annotations for location_config from Dict to dict in export managers
backup.py Modified import for Iterable and updated type hints for _parse_segment_frames
cache.py Replaced Tuple with tuple in type annotations, updated imports from collections.abc
cloud_provider.py Updated method signatures to use list[str], dict, and refined type hints
field_validation.py Changed Sequence import from typing to collections.abc
filters.py Updated type hints for lookup fields and method return types
frame_provider.py Modernized type annotations, replaced Type with type
lazy_list.py Enhanced type hints and added wrapper methods for list operations
location.py Updated get_location_configuration type hints
media_extractors.py Refined type annotations for iterators and method signatures
migrations/0083_move_to_segment_chunks.py Updated Iterable import
mixins.py Changed type hints for export_dataset_v1 method
models.py Updated JobQuerySet method type hints
permissions.py Modernized type annotations for various methods
schema.py Updated _copy_serializer type hint
serializers.py Refined type hints and imports
task.py Updated type aliases and method signatures
tests/utils.py Modernized filter_dict type annotations
utils.py Updated imports and type hints for various functions
view_utils.py Replaced Type with type in method signatures
views.py Updated type hints for list and dictionary types

Poem

🐰 Hop, hop, type annotations dance,
Transforming code with a rabbit's glance
From Tuple to tuple, we refine
Our Python types now brightly shine!
A type-safe leap across the page 🌈

Tip

CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command @coderabbitai generate docstrings to have CodeRabbit automatically generate docstrings for your pull request. We would love to hear your feedback on Discord.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 54534e8 and 8c63d5e.

📒 Files selected for processing (23)
  • cvat/apps/engine/background.py (3 hunks)
  • cvat/apps/engine/backup.py (2 hunks)
  • cvat/apps/engine/cache.py (6 hunks)
  • cvat/apps/engine/cloud_provider.py (12 hunks)
  • cvat/apps/engine/field_validation.py (1 hunks)
  • cvat/apps/engine/filters.py (6 hunks)
  • cvat/apps/engine/frame_provider.py (9 hunks)
  • cvat/apps/engine/lazy_list.py (1 hunks)
  • cvat/apps/engine/location.py (2 hunks)
  • cvat/apps/engine/media_extractors.py (6 hunks)
  • cvat/apps/engine/migrations/0083_move_to_segment_chunks.py (1 hunks)
  • cvat/apps/engine/migrations/0084_honeypot_support.py (1 hunks)
  • cvat/apps/engine/mixins.py (2 hunks)
  • cvat/apps/engine/models.py (2 hunks)
  • cvat/apps/engine/permissions.py (5 hunks)
  • cvat/apps/engine/schema.py (1 hunks)
  • cvat/apps/engine/serializers.py (10 hunks)
  • cvat/apps/engine/task.py (12 hunks)
  • cvat/apps/engine/task_validation.py (1 hunks)
  • cvat/apps/engine/tests/utils.py (2 hunks)
  • cvat/apps/engine/utils.py (3 hunks)
  • cvat/apps/engine/view_utils.py (3 hunks)
  • cvat/apps/engine/views.py (4 hunks)
✅ Files skipped from review due to trivial changes (5)
  • cvat/apps/engine/task_validation.py
  • cvat/apps/engine/field_validation.py
  • cvat/apps/engine/backup.py
  • cvat/apps/engine/schema.py
  • cvat/apps/engine/permissions.py
🔇 Additional comments (32)
cvat/apps/engine/lazy_list.py (2)

5-5: Adopt consistent imports from collections.abc
Switching from the legacy “typing.Iterator” to “collections.abc.Iterator” is aligned with modern Python practices. This change is good as it reduces reliance on deprecated imports.


8-8: Confirm no stale references to typing.Iterator remain
Please ensure that all other references to “Iterator” in this file (or elsewhere) have been updated to avoid confusion. A quick search could confirm there are no references left in docstrings or untested utility functions.

✅ Verification successful

No stale references to typing.Iterator found - imports updated correctly

The file has been properly updated to use collections.abc.Iterator instead of typing.Iterator. The only reference to Iterator in lazy_list.py is the correct import from collections.abc import Iterator and its usage in type hints. No stale references or inconsistencies were found.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Searching for any leftover references to typing.Iterator
rg -A 2 $'typing.Iterator'

Length of output: 25


Script:

#!/bin/bash
# Search for any references to Iterator, including in docstrings and type hints
rg -i "iterator" cvat/apps/engine/lazy_list.py

Length of output: 345


Script:

#!/bin/bash
# Search for Iterator in type hints and imports using AST
ast-grep --pattern 'Iterator'

Length of output: 3621


Script:

#!/bin/bash
# Search for any references to Iterator in Python files
rg "Iterator" --type py

Length of output: 4296

cvat/apps/engine/migrations/0084_honeypot_support.py (1)

4-4: Use of “Collection” from collections.abc is consistent
Importing “Collection” from collections.abc is the recommended approach in newer Python versions. This promotes consistency with other updates removing deprecated typing imports.

cvat/apps/engine/location.py (2)

18-22: Successful migration to built-in dict types
Changing “Dict[str, Any]” → “dict[str, Any]” in both parameter and return annotations aligns with modern type hinting. The function signature is still clear and consistent with the rest of the PR’s changes.


6-6: Confirm removal of old “Dict” references
Although “Any, Union, Optional” remain, ensure that “Dict” was removed intentionally and is no longer used. If leftover references exist, they can cause confusion.

✅ Verification successful

Let me verify if there are any remaining references to Dict specifically in the cvat/apps/engine/location.py file or any dependencies that might be affected by its removal.


Let me check the actual usage of Dict in the engine directory files to determine if the removal from location.py impacts them or if they have independent imports.


Removal of Dict import is safe and correct
The removal of Dict from location.py is intentional and safe. The files that use Dict in the engine directory either use it from collections (OrderedDict) or have their own custom dictionary implementations. No files are importing Dict from the typing module through location.py.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
#!/bin/bash
# Check for leftover references to typing.Dict
rg -A 2 $'from typing import Dict'

Length of output: 138


Script:

#!/bin/bash
# Check for Dict usage in location.py
rg -l "Dict" "cvat/apps/engine/location.py"

# Check for Dict type annotations in location.py using ast-grep
ast-grep --pattern 'Dict[' "cvat/apps/engine/location.py"

# Check for any Dict imports or usage in the engine directory
rg -l "Dict" "cvat/apps/engine/"

Length of output: 818


Script:

#!/bin/bash
# Check for Dict imports and usage in the relevant engine files
rg -A 2 "Dict" "cvat/apps/engine/serializers.py" "cvat/apps/engine/frame_provider.py" "cvat/apps/engine/filters.py"

# Check if location.py is imported in these files
rg -l "from.*location import" "cvat/apps/engine/"

Length of output: 1938

cvat/apps/engine/view_utils.py (3)

26-28: Modernizing type hints to built-in generics
Switching to “type[HttpResponse]”, “type[Serializer]”, and “type[HttpRequest]” accurately reflects the newer Python syntax. This makes the function signature more readable.


57-57: Refactor ensures clarity
Updating “serializer_class: Type[Serializer]” → “serializer_class: type[Serializer]” maintains consistency with other type hint changes. This is a straightforward yet effective improvement.


7-7: Check for any new import collisions
Removing or consolidating typing imports is fine, but verify that “Optional” is the only necessity here. Sometimes leftover or replaced imports can cause collisions or lint warnings.

✅ Verification successful

Let me check for any potential import collisions or unused imports by searching for Optional usage in the file.


Import of Optional is correctly used and necessary
The Optional type from typing is actively used in the function signature of make_paginated_response() for three parameters: response_type, serializer_type, and request. There are no import collisions or unused imports in this file.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Searching for references to typing union usage
rg -A 2 $'from typing import Union'

Length of output: 365


Script:

#!/bin/bash
# Search for Optional usage in the specific file
rg -l "Optional" "cvat/apps/engine/view_utils.py"

# Show the content of the file to analyze Optional usage
cat "cvat/apps/engine/view_utils.py"

Length of output: 3351

cvat/apps/engine/migrations/0083_move_to_segment_chunks.py (1)

4-4: LGTM: Import modernization

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

cvat/apps/engine/tests/utils.py (2)

5-5: LGTM: Updated collection type imports

The change to import collection types from collections.abc follows PEP 585 recommendations.


182-183: LGTM: Modern type hint syntax

Updated type hint from Dict[str, Any] to dict[str, Any] maintains the same semantics while using modern Python type annotation syntax.

cvat/apps/engine/utils.py (3)

10-10: LGTM: Comprehensive collection type imports

The change consolidates collection type imports from collections.abc, following PEP 585 recommendations.


266-266: LGTM: Updated query_params type hint

Updated type hint from Dict[str, str] to dict[str, str] for better readability and modern Python standards.


285-285: LGTM: Updated return type hint

The return type hint update to dict[str, str] aligns with modern Python type annotation practices.

cvat/apps/engine/filters.py (1)

6-6: LGTM! Type hint modernization looks good.

The changes consistently update type hints to use built-in types instead of those from the typing module, following PEP 585 recommendations. This improves code readability while maintaining the same type safety.

Also applies to: 29-29, 138-138, 195-195, 366-366

cvat/apps/engine/mixins.py (1)

11-11: LGTM! Type hint modernization is consistent.

The changes align with the codebase-wide effort to use built-in types and collections.abc for type hints, following modern Python practices.

Also applies to: 428-428

cvat/apps/engine/frame_provider.py (1)

14-14: LGTM! Type hint modernization is thorough and consistent.

The changes systematically update type hints across the frame provider implementation to use:

  • collections.abc for abstract base classes
  • Built-in types instead of typing module equivalents
  • Proper generic type parameters

This improves code readability while maintaining type safety.

Also applies to: 53-53, 88-88, 108-108, 451-451, 523-523, 576-576

cvat/apps/engine/background.py (1)

173-173: LGTM: Type hint modernization

The change from Dict[str, Any] to dict[str, Any] aligns with PEP 585, which allows using built-in collection types as generic types. This is a safe change that improves code readability.

cvat/apps/engine/cloud_provider.py (1)

237-237: LGTM: Consistent type hint modernization

The changes replace deprecated typing module imports with their built-in equivalents:

  • List[str]list[str]
  • Dictdict
    This modernization aligns with PEP 585 and improves code consistency.

Also applies to: 251-251, 279-279, 342-342, 371-371

cvat/apps/engine/cache.py (1)

68-69: LGTM: Comprehensive type hint modernization

The changes systematically update all type hints to use built-in types:

  • Tupletuple
  • Typetype
  • Complex nested type hints are updated correctly while maintaining their structure

This modernization improves code consistency and readability while following PEP 585 guidelines.

Also applies to: 628-628, 954-954, 964-964

cvat/apps/engine/models.py (2)

13-13: LGTM: Import changes follow modern Python practices

The change from typing to collections.abc for Collection and Sequence imports aligns with PEP 585 recommendations for using built-in collection types.

Also applies to: 16-16


828-828: LGTM: Type annotation updated to use built-in dict type

The parameter type annotation change from Dict[str, Any] to dict[str, Any] follows PEP 585 recommendations for using built-in types in annotations.

cvat/apps/engine/media_extractors.py (2)

18-19: LGTM: Import changes follow modern Python practices

The changes properly organize imports by:

  1. Using collections.abc for abstract collection types
  2. Using AbstractContextManager from contextlib
  3. Cleaning up typing imports

Also applies to: 22-22


613-613: LGTM: Return type annotations updated to use built-in tuple type

The return type annotations have been updated to use the built-in tuple type instead of Tuple from typing, following PEP 585 recommendations.

Also applies to: 674-674, 681-681, 772-772

cvat/apps/engine/task.py (2)

13-13: LGTM: Import changes follow modern Python practices

The changes properly organize imports by:

  1. Using collections.abc for Iterator and Sequence
  2. Cleaning up typing imports to remove redundant types

Also applies to: 18-18


81-81: LGTM: Type annotations updated to use built-in collection types

All type annotations have been consistently updated to use built-in collection types (list, dict) instead of their typing module counterparts (List, Dict), following PEP 585 recommendations.

Also applies to: 95-95, 98-98, 308-308, 347-347, 386-386, 459-459, 483-483, 515-515, 540-540, 561-561

cvat/apps/engine/serializers.py (4)

372-374: LGTM! Type annotation updates follow best practices

The change from Dict[str, Any] to dict[str, Any] aligns with PEP 585, which allows using built-in collection types in annotations. This improves code readability while maintaining the same functionality.


488-488: LGTM! Consistent type annotation updates

The changes from List[Dict[str, Any]] to list[dict[str, Any]] and Iterable[Dict[str, Any]] to Iterable[dict[str, Any]] maintain consistency with the type annotation modernization across the codebase.

Also applies to: 539-539


3275-3275: LGTM! Type hints modernization for storage-related functions

The type annotation updates in storage-related functions follow the same pattern of using built-in types, maintaining consistency throughout the codebase.

Also applies to: 3330-3330


3423-3423: LGTM! Return type annotations updated consistently

The return type annotations in the to_representation methods have been updated to use built-in types, following the same modernization pattern.

Also applies to: 3464-3464, 3504-3504

cvat/apps/engine/views.py (2)

Line range hint 3572-3591: LGTM! Well-structured queue job retrieval implementation

The function implementation is clean with proper error handling and type annotations. It efficiently retrieves jobs from a queue while filtering for a specific user.


Line range hint 3592-3604: LGTM! Clear and well-documented implementation

The function has:

  • Clear docstring explaining purpose and return values
  • Proper type annotations
  • Efficient reuse of _get_rq_jobs_from_queue()

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 generate docstrings to generate docstrings for this PR. (Beta)
  • @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.

@SpecLad SpecLad merged commit 12f886c into cvat-ai:develop Dec 20, 2024
35 checks passed
@SpecLad SpecLad deleted the deprecated-typing branch December 20, 2024 17:33
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