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

feat: added wikipedia local tools #222

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dhruv1710
Copy link

@dhruv1710 dhruv1710 commented Jun 27, 2024

User description

Other problems and make checks mentioned in previous pull request resolved.


PR Type

Enhancement, Other


Description

  • Added new action to scrape Wikipedia content, including request and response schemas using Pydantic.
  • Defined and registered a new Wikipedia tool.
  • Reorganized imports and reformatted code for better readability and consistency across multiple files.
  • Adjusted exception handling logic in catch_all_exceptions.py.
  • Minor formatting improvements in various files.

Changes walkthrough 📝

Relevant files
Enhancement
7 files
catch_all_exceptions.py
Code cleanup and import reorganization in exception handling.

composio/core/cls/catch_all_exceptions.py

  • Reorganized imports for better readability.
  • Minor formatting changes for consistency.
  • Adjusted exception handling logic.
  • +14/-12 
    scrape_wikipedia.py
    New action for scraping Wikipedia content.                             

    composio/local_tools/wikipedia/actions/scrape_wikipedia.py

  • Added new action to scrape Wikipedia content.
  • Defined request and response schemas using Pydantic.
  • Implemented Wikipedia content scraping logic.
  • +46/-0   
    __init__.py
    Import reorganization and click group reformatting.           

    composio/cli/init.py

  • Reorganized imports for better readability.
  • Reformatted click group definition.
  • +13/-4   
    local_handler.py
    Registered Wikipedia tool in local handler.                           

    composio/client/local_handler.py

    • Registered new Wikipedia tool.
    +2/-0     
    tool.py
    New Wikipedia tool definition and actions.                             

    composio/local_tools/wikipedia/tool.py

  • Added new Wikipedia tool definition.
  • Defined actions for the Wikipedia tool.
  • +15/-0   
    __init__.py
    Import Wikipedia content action.                                                 

    composio/local_tools/wikipedia/actions/init.py

    • Added import for Wikipedia content action.
    +1/-0     
    __init__.py
    Import Wikipedia tool.                                                                     

    composio/local_tools/wikipedia/init.py

    • Added import for Wikipedia tool.
    +1/-0     
    Formatting
    3 files
    __init__.py
    Code reformatting and consistency improvements.                   

    composio/client/init.py

  • Reformatted code for better readability.
  • Adjusted method definitions for consistency.
  • +9/-5     
    actions.py
    Minor formatting improvement.                                                       

    composio/cli/actions.py

    • Added a newline for better code separation.
    +1/-0     
    did_you_mean.py
    Minor formatting improvement.                                                       

    composio/core/cls/did_you_mean.py

    • Added a newline for better code separation.
    +1/-0     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @codiumai-pr-agent-pro codiumai-pr-agent-pro bot added enhancement New feature or request other labels Jun 27, 2024
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 3
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review Possible Bug:
    The execute method in scrape_wikipedia.py attempts to import wikipediaapi inside the method. This could lead to performance issues and unexpected failures if the module is not available. Consider moving imports to the top of the file.
    Error Handling:
    In scrape_wikipedia.py, the error handling could be improved by providing more specific error messages or handling different types of exceptions differently.
    Code Consistency:
    The reformatting in various files like catch_all_exceptions.py and __init__.py files should be reviewed to ensure it follows the project's coding standards.

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Performance
    Use a dictionary for efficient mapping and lookup in class methods

    Consider using a dictionary to map app names to their properties instead of multiple
    if-else or switch-case statements in methods like from_app, from_action, and
    from_app_and_action in the Action class. This will make the code cleaner and more
    efficient.

    composio/client/enums.py [467-472]

     @classmethod
     def from_app(cls, name: str) -> "Action":
         """Create Action type enum from app name."""
    -    for action in cls:
    -        if name == action.app:
    -            return action
    +    action_map = {action.app: action for action in cls}
    +    if name in action_map:
    +        return action_map[name]
         raise ValueError(f"No action type found for name `{name}`")
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: The suggestion to use a dictionary for mapping is excellent for improving code efficiency and readability. It simplifies the logic and enhances performance.

    9
    Use a set for faster membership checking in is_local method

    Refactor the is_local method in the App class to use a set for checking if the app is
    local. This will improve the performance as checking membership in a set is faster than
    checking membership in a list.

    composio/client/enums.py [378-389]

     def is_local(self) -> bool:
         """If the app is local."""
    -    return self.value.lower() in [
    +    local_apps = {
             "mathematical",
             "localworkspace",
             ...
             "wikipedia",
    -    ]
    +    }
    +    return self.value.lower() in local_apps
     
    Suggestion importance[1-10]: 8

    Why: Using a set for membership checking is a good performance optimization. This change is straightforward and improves efficiency without altering the method's functionality.

    8
    Move the import of wikipediaapi to the top of the file to improve code clarity and performance

    Replace the dynamic import of wikipediaapi with a top-level import to improve performance
    and avoid potential issues with import errors not being caught where expected.

    composio/local_tools/wikipedia/actions/scrape_wikipedia.py [31-34]

    -# pylint: disable=import-outside-toplevel
     import wikipediaapi
    -# pylint: enable=import-outside-toplevel
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Moving the import to the top of the file improves code clarity and performance by avoiding dynamic imports. However, it is a minor optimization.

    7
    Possible bug
    Add error handling for missing DNS configuration to prevent runtime errors

    Add error handling for the case where config.get("dns") returns None, which could cause
    the Sentry SDK initialization to fail.

    composio/core/cls/catch_all_exceptions.py [16]

    +dsn_value = config.get("dns")
    +if dsn_value is None:
    +    raise ValueError("DNS configuration is missing.")
     sentry_sdk.init(
    -    dsn=config.get("dns"), traces_sample_rate=1.0, profiles_sample_rate=1.0
    +    dsn=dsn_value, traces_sample_rate=1.0, profiles_sample_rate=1.0
     )
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: This suggestion addresses a potential runtime error by adding error handling for a missing DNS configuration, which is crucial for robust code.

    9
    Add error handling to is_local method to manage non-string value attributes

    Implement error handling for the is_local method in the App class to handle potential
    issues with the value attribute not being a string, which could lead to an AttributeError
    when calling lower().

    composio/client/enums.py [378-389]

     def is_local(self) -> bool:
         """If the app is local."""
    -    return self.value.lower() in [
    -        "mathematical",
    -        "localworkspace",
    -        ...
    -        "wikipedia",
    -    ]
    +    try:
    +        return self.value.lower() in {
    +            "mathematical",
    +            "localworkspace",
    +            ...
    +            "wikipedia",
    +        }
    +    except AttributeError:
    +        raise ValueError("App value must be a string")
     
    Suggestion importance[1-10]: 6

    Why: Adding error handling is a good practice to prevent potential runtime errors. However, the likelihood of value not being a string in this context seems low, making this a minor improvement.

    6
    Enhancement
    Simplify the isinstance checks by combining them into a single call

    Combine the isinstance checks into a single call to simplify the condition and improve
    readability.

    composio/core/cls/catch_all_exceptions.py [56-58]

    -if (
    -    isinstance(exc, ValueError) or isinstance(exc, SystemExit)
    -) and sentry_sdk.is_initialized():
    +if isinstance(exc, (ValueError, SystemExit)) and sentry_sdk.is_initialized():
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: This suggestion improves readability and simplifies the code without altering functionality. It is a minor enhancement but beneficial for code clarity.

    8
    Maintainability
    Replace tuple inheritance with dataclass for better structure and maintainability

    Replace the tuple inheritance in Tag and Action classes with a more suitable data
    structure like a dataclass for better readability and maintainability. Tuples are
    immutable and should be used when the entity represents a collection of heterogeneous
    (different) data types, but here a more structured approach could be beneficial.

    composio/client/enums.py [10-443]

    -class Tag(tuple, Enum):
    +from dataclasses import dataclass
    +@dataclass
    +class Tag(Enum):
         """App tags."""
         ...
    -class Action(tuple, Enum):
    +@dataclass
    +class Action(Enum):
         """App action."""
         ...
     
    Suggestion importance[1-10]: 7

    Why: The suggestion to use dataclasses instead of tuples can improve readability and maintainability. However, it may require significant refactoring and testing to ensure compatibility with existing code.

    7

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    None yet

    1 participant