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

improve_typing #528

Merged
merged 8 commits into from
Jul 22, 2024
Merged

improve_typing #528

merged 8 commits into from
Jul 22, 2024

Conversation

JarbasAl
Copy link
Member

@JarbasAl JarbasAl commented Jul 22, 2024

Summary by CodeRabbit

  • New Features

    • Improved type annotations and return types across multiple services for better clarity and type safety.
    • Streamlined IntentMatch usage by importing it from a centralized module.
  • Bug Fixes

    • Enhanced method clarity and predictability with updated return types to indicate optional returns.
  • Documentation

    • Updated class docstrings to reflect correct branding and improve overall documentation alignment.
  • Chores

    • Updated dependency version for ovos-plugin-manager to incorporate new features and fixes.

@JarbasAl JarbasAl added the refactor code refactor without functional changes label Jul 22, 2024
Copy link

coderabbitai bot commented Jul 22, 2024

Warning

Rate limit exceeded

@JarbasAl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 31 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Commits

Files that changed from the base of the PR and between 3c27ac6 and 328734a.

Walkthrough

The recent updates across multiple files in the ovos_core.intent_services module primarily enhance code clarity and maintainability. A key change is the transition from a locally defined IntentMatch namedtuple to an imported class from ovos_plugin_manager.templates.pipeline. This, coupled with the addition and refinement of type annotations throughout various methods, significantly improves type safety and documentation. Overall, these modifications streamline intent handling processes and align the codebase with modern Python standards.

Changes

Files Change Summary
ovos_core/intent_services/__init__.py Removed IntentMatch namedtuple, imported from ovos_plugin_manager.templates.pipeline, updated IntentService docstring.
ovos_core/intent_services/adapt_service.py Updated IntentMatch import and simplified usage in take_best function.
ovos_core/intent_services/commonqa_service.py Added Optional type hint to match method, updated IntentMatch import.
ovos_core/intent_services/converse_service.py Changed return type of converse_with_skills to Optional[IntentMatch], updated import.
ovos_core/intent_services/fallback_service.py Enhanced type annotations for methods, updated IntentMatch import.
ovos_core/intent_services/ocp_service.py Standardized IntentMatch usage, added return type annotations.
ovos_core/intent_services/padacioso_service.py Updated IntentMatch import and return type in _match_level method.
ovos_core/intent_services/padatious_service.py Simplified import of IntentMatch, updated return statement in _match_level.
ovos_core/intent_services/stop_service.py Added type hints for several methods, enhancing readability and type safety.
requirements/requirements.txt Updated ovos-plugin-manager version constraint from >=0.0.25 to >=0.0.26a33.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant IntentService
    participant IntentMatch
    participant FallbackService

    User->>IntentService: Request Intent
    IntentService->>IntentMatch: Create IntentMatch
    IntentMatch->>IntentService: Return Match Information
    IntentService->>FallbackService: Handle Fallback
    FallbackService->>User: Provide Response
Loading

🐇 In the code where rabbits play,
Intent matching finds its way.
From imports new, the paths are clear,
With type hints, there's nothing to fear!
Hop along, let's celebrate,
A sprightly code, oh, isn't it great? 🎉


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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as 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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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.

@JarbasAl JarbasAl added this to the 0.0.8 milestone Jul 22, 2024
Copy link

@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, codebase verification and nitpick comments (1)
ovos_core/intent_services/padacioso_service.py (1)

12-13: Incorrect import path for IntentMatch.

The IntentMatch named tuple is defined in mycroft/skills/intent_services/base.py, not in ovos_plugin_manager.templates.pipeline.

  • Incorrect Import:

    from ovos_plugin_manager.templates.pipeline import IntentMatch
  • Correct Import:

    from mycroft.skills.intent_services.base import IntentMatch
Analysis chain

Verify the new import path for IntentMatch.

Ensure that IntentMatch is correctly defined in ovos_plugin_manager.templates.pipeline.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the new import path for `IntentMatch`.

# Test: Search for the definition of `IntentMatch` in the specified module.
rg --type python --pattern $'class IntentMatch' ovos_plugin_manager/templates/pipeline.py

Length of output: 502


Script:

#!/bin/bash
# Description: Verify the new import path for `IntentMatch`.

# Test: Search for the definition of `IntentMatch` in the specified module.
rg 'class IntentMatch' ovos_plugin_manager/templates/pipeline.py

Length of output: 148


Script:

#!/bin/bash
# Description: Verify the new import path for `IntentMatch`.

# Test: Search for the definition of `IntentMatch` across the entire repository.
rg 'class IntentMatch'

Length of output: 24


Script:

#!/bin/bash
# Description: Verify the new import path for `IntentMatch`.

# Test: Search for any mention of `IntentMatch` across the entire repository.
rg 'IntentMatch'

Length of output: 5767


Script:

#!/bin/bash
# Description: Verify the definition of `IntentMatch`.

# Test: Search for the definition of `IntentMatch` in the specified file.
rg 'IntentMatch' mycroft/skills/intent_services/base.py

Length of output: 95

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b603751 and bb456c0.

Files selected for processing (10)
  • ovos_core/intent_services/init.py (2 hunks)
  • ovos_core/intent_services/adapt_service.py (2 hunks)
  • ovos_core/intent_services/commonqa_service.py (3 hunks)
  • ovos_core/intent_services/converse_service.py (3 hunks)
  • ovos_core/intent_services/fallback_service.py (3 hunks)
  • ovos_core/intent_services/ocp_service.py (8 hunks)
  • ovos_core/intent_services/padacioso_service.py (3 hunks)
  • ovos_core/intent_services/padatious_service.py (2 hunks)
  • ovos_core/intent_services/stop_service.py (9 hunks)
  • requirements/requirements.txt (1 hunks)
Files skipped from review due to trivial changes (1)
  • ovos_core/intent_services/adapt_service.py
Additional context used
Ruff
ovos_core/intent_services/converse_service.py

314-314: Undefined name Optional

(F821)

ovos_core/intent_services/ocp_service.py

414-414: f-string without any placeholders

Remove extraneous f prefix

(F541)


441-441: f-string without any placeholders

Remove extraneous f prefix

(F541)


458-458: f-string without any placeholders

Remove extraneous f prefix

(F541)


469-469: f-string without any placeholders

Remove extraneous f prefix

(F541)


493-493: f-string without any placeholders

Remove extraneous f prefix

(F541)


1093-1093: f-string without any placeholders

Remove extraneous f prefix

(F541)

Additional comments not posted (24)
requirements/requirements.txt (1)

11-11: Verify compatibility with the updated ovos-plugin-manager version.

The version update from >=0.0.25 to >=0.0.26a33 might introduce new features or fixes. Ensure that the rest of the codebase is compatible with this new version.

Verification successful

Compatibility with the updated ovos-plugin-manager version verified.

The ovos-plugin-manager package is only listed as a core package and protected from uninstallation in the ovos_core/skill_installer.py file. There are no direct interactions with its API in the provided context, suggesting that the version update should not cause compatibility issues.

  • ovos_core/skill_installer.py: Verified usage and no direct API interactions found.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify compatibility with the updated `ovos-plugin-manager` version.

# Test: List all dependencies and check for compatibility issues.
pip list

Length of output: 81


Script:

#!/bin/bash
# Search for any usage of `ovos-plugin-manager` in the codebase to check for potential compatibility issues.
rg 'ovos-plugin-manager'

Length of output: 193


Script:

#!/bin/bash
# List all the Python files to understand where `ovos-plugin-manager` might be used.
fd --extension py

Length of output: 10164


Script:

#!/bin/bash
# Extract and inspect the usage of `ovos-plugin-manager` in `ovos_core/skill_installer.py` to verify compatibility.
rg 'ovos-plugin-manager' -A 10 ovos_core/skill_installer.py

Length of output: 577


Script:

#!/bin/bash
# Search for all occurrences of `ovos-plugin-manager` in `ovos_core/skill_installer.py` to gather more context.
rg 'ovos-plugin-manager' ovos_core/skill_installer.py -A 20

Length of output: 982

ovos_core/intent_services/fallback_service.py (5)

Line range hint 162-196:
LGTM!

The return type annotation Optional[IntentMatch] is appropriate and enhances type safety.


199-202: LGTM!

The return type annotation Optional[IntentMatch] is appropriate and enhances type safety.


204-207: LGTM!

The return type annotation Optional[IntentMatch] is appropriate and enhances type safety.


209-212: LGTM!

The return type annotation Optional[IntentMatch] is appropriate and enhances type safety.


18-24: Verify the new import path for IntentMatch.

Ensure that IntentMatch is correctly defined in ovos_plugin_manager.templates.pipeline.

ovos_core/intent_services/padacioso_service.py (1)

95-97: LGTM!

The updated return type for _match_level simplifies the code and improves clarity.

ovos_core/intent_services/stop_service.py (6)

45-45: Type Annotation Added for get_active_skills

The addition of the type annotation for the message parameter enhances clarity and type safety.


55-55: Type Annotation Added for _collect_stop_skills

The addition of the type annotation for the message parameter enhances clarity and type safety.


99-99: Type Annotations Added for stop_skill

The addition of type annotations for the skill_id and message parameters enhances clarity and type safety.


119-119: Type Annotations and Return Type Added for match_stop_high

The addition of type annotations for the utterances, lang, and message parameters, along with the return type annotation, enhances clarity and type safety.


169-169: Type Annotations and Return Type Added for match_stop_medium

The addition of type annotations for the utterances, lang, and message parameters, along with the return type annotation, enhances clarity and type safety.


197-197: Type Annotations and Return Type Added for match_stop_low

The addition of type annotations for the utterances, lang, and message parameters, along with the return type annotation, enhances clarity and type safety.

ovos_core/intent_services/padatious_service.py (1)

58-58: Updated Usage of IntentMatch in _match_level

The change to use IntentMatch directly from ovos_plugin_manager.templates.pipeline improves modularity and reduces dependency on the specific module path.

ovos_core/intent_services/commonqa_service.py (1)

79-79: Type Annotation and Updated Usage of IntentMatch in match

The addition of the return type annotation and the change to use IntentMatch directly from ovos_plugin_manager.templates.pipeline improve clarity, type safety, and modularity.

ovos_core/intent_services/converse_service.py (1)

Line range hint 314-343:
LGTM! But verify the method usage in the codebase.

The code changes are approved.

However, ensure that all method calls to converse_with_skills match the new return type.

Verification successful

Ensure future usage of converse_with_skills handles the new return type correctly.

The current method calls to converse_with_skills in the codebase do not handle its return value, so there are no immediate issues with the return type handling. However, it is important to monitor any future usage to ensure the Optional[IntentMatch] return type is correctly handled.

  • ovos_core/intent_services/converse_service.py
  • ovos_core/intent_services/__init__.py
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all method calls to `converse_with_skills` match the new return type.

# Test: Search for the method usage. Expect: Only occurrences of the new return type.
rg --type python -A 5 $'converse_with_skills'

Length of output: 75


Script:

#!/bin/bash
# Description: Verify all method calls to `converse_with_skills` match the new return type.

# Test: Search for the method usage. Expect: Only occurrences of the new return type.
rg --type py -A 5 $'converse_with_skills'

Length of output: 1601


Script:

#!/bin/bash
# Description: Extract more context around each usage of `converse_with_skills` to verify return type handling.

# Extract 10 lines of context before and after each usage of `converse_with_skills`
rg --type py -C 10 $'converse_with_skills'

Length of output: 5687

ovos_core/intent_services/__init__.py (2)

33-33: LGTM!

The import change aligns with the goal of centralizing the IntentMatch definition.


37-37: LGTM!

The docstring update reflects the current branding and context.

ovos_core/intent_services/ocp_service.py (6)

24-24: LGTM!

The import change aligns with the goal of centralizing the IntentMatch definition.


353-353: LGTM!

The return type annotation improves type safety and readability.


394-394: LGTM!

The return type annotation improves type safety and readability.


423-423: LGTM!

The return type annotation improves type safety and readability.


450-450: LGTM!

The return type annotation improves type safety and readability.


1073-1073: LGTM!

The return type annotation improves type safety and readability.

ovos_core/intent_services/converse_service.py Outdated Show resolved Hide resolved
Copy link

@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: 6

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between bb456c0 and 3c27ac6.

Files selected for processing (8)
  • ovos_core/intent_services/adapt_service.py (2 hunks)
  • ovos_core/intent_services/commonqa_service.py (3 hunks)
  • ovos_core/intent_services/converse_service.py (4 hunks)
  • ovos_core/intent_services/fallback_service.py (3 hunks)
  • ovos_core/intent_services/ocp_service.py (8 hunks)
  • ovos_core/intent_services/padacioso_service.py (2 hunks)
  • ovos_core/intent_services/padatious_service.py (2 hunks)
  • ovos_core/intent_services/stop_service.py (10 hunks)
Files skipped from review due to trivial changes (1)
  • ovos_core/intent_services/adapt_service.py
Files skipped from review as they are similar to previous changes (6)
  • ovos_core/intent_services/commonqa_service.py
  • ovos_core/intent_services/converse_service.py
  • ovos_core/intent_services/fallback_service.py
  • ovos_core/intent_services/padacioso_service.py
  • ovos_core/intent_services/padatious_service.py
  • ovos_core/intent_services/stop_service.py
Additional context used
Ruff
ovos_core/intent_services/ocp_service.py

414-414: f-string without any placeholders

Remove extraneous f prefix

(F541)


441-441: f-string without any placeholders

Remove extraneous f prefix

(F541)


458-458: f-string without any placeholders

Remove extraneous f prefix

(F541)


469-469: f-string without any placeholders

Remove extraneous f prefix

(F541)


493-493: f-string without any placeholders

Remove extraneous f prefix

(F541)


1093-1093: f-string without any placeholders

Remove extraneous f prefix

(F541)

Additional comments not posted (6)
ovos_core/intent_services/ocp_service.py (6)

353-353: LGTM! The type annotation improves type safety.

The return type annotation Optional[IntentMatch] is a valuable addition.


394-394: LGTM! The type annotation improves type safety.

The return type annotation Optional[IntentMatch] is a valuable addition.


423-423: LGTM! The type annotation improves type safety.

The return type annotation Optional[IntentMatch] is a valuable addition.


450-450: LGTM! The type annotation improves type safety.

The return type annotation Optional[IntentMatch] is a valuable addition.


1073-1073: LGTM! The type annotation improves type safety.

The return type annotation Optional[IntentMatch] is a valuable addition.


388-388: Remove unnecessary f prefix.

The f prefix is not needed as there are no placeholders in the string.

-    intent_type=f'ocp:{match["name"]}',
+    intent_type='ocp:{match["name"]}',

Likely invalid or redundant comment.

ovos_core/intent_services/ocp_service.py Outdated Show resolved Hide resolved
ovos_core/intent_services/ocp_service.py Outdated Show resolved Hide resolved
ovos_core/intent_services/ocp_service.py Outdated Show resolved Hide resolved
ovos_core/intent_services/ocp_service.py Outdated Show resolved Hide resolved
ovos_core/intent_services/ocp_service.py Outdated Show resolved Hide resolved
ovos_core/intent_services/ocp_service.py Outdated Show resolved Hide resolved
JarbasAl and others added 6 commits July 22, 2024 18:51
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link

codecov bot commented Jul 22, 2024

Codecov Report

Attention: Patch coverage is 95.23810% with 3 lines in your changes missing coverage. Please review.

Project coverage is 75.70%. Comparing base (b603751) to head (328734a).

Files Patch % Lines
ovos_core/intent_services/ocp_service.py 88.88% 2 Missing ⚠️
ovos_core/intent_services/stop_service.py 91.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #528      +/-   ##
==========================================
- Coverage   75.72%   75.70%   -0.02%     
==========================================
  Files          15       15              
  Lines        3044     3042       -2     
==========================================
- Hits         2305     2303       -2     
  Misses        739      739              
Flag Coverage Δ
unittests 75.70% <95.23%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JarbasAl JarbasAl merged commit 41fb56b into dev Jul 22, 2024
7 of 8 checks passed
@JarbasAl JarbasAl deleted the refactor/typing_updates branch July 22, 2024 18:30
@coderabbitai coderabbitai bot mentioned this pull request Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor code refactor without functional changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant