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

fix aask() got an unexpected keyword argument 'images' #1264

Merged
merged 1 commit into from
May 16, 2024

Conversation

jainkunal
Copy link
Contributor

@jainkunal jainkunal commented May 11, 2024

User description

This fixes the error when running software_company with is_human flag True for ProjectManager.

The issue is that BaseLLM.aask takes in more keyword arguments than handled by HumanProvider.aask and that results in the following error.

aask() got an unexpected keyword argument 'images'

Tested the code after the fix.


PR Type

Bug fix


Description

  • Added support for flexible keyword arguments in the aask method of HumanProvider class. This prevents errors when unexpected keyword arguments are passed, enhancing the method's robustness.

Changes walkthrough 📝

Relevant files
Bug fix
human_provider.py
Add flexible keyword argument handling to `aask` method   

metagpt/provider/human_provider.py

  • Added **kwargs to the aask method to handle additional unexpected
    keyword arguments.
  • +1/-0     

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

    Copy link

    PR Description updated to latest commit (d4bd668)

    Copy link

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    1, because the change is minimal and straightforward, involving only the addition of **kwargs to handle additional unexpected keyword arguments in a method.

    🧪 Relevant tests

    No

    ⚡ Possible issues

    No

    🔒 Security concerns

    No

    @jainkunal jainkunal changed the title fix ask() got an unexpected keyword argument 'images' fix aask() got an unexpected keyword argument 'images' May 11, 2024
    Copy link

    PR Code Suggestions ✨

    CategorySuggestions                                                                                                                                                       
    Possible issue
    Ensure that arbitrary keyword arguments are properly handled or passed through.

    **The addition of **kwargs in the function signature of aask allows for arbitrary keyword
    arguments, which might lead to unexpected behavior if not handled properly. If the
    intention is to pass these additional keyword arguments to the ask method, explicitly pass
    **kwargs to it. If not, consider removing kwargs or handling it explicitly to avoid
    potential bugs.

    metagpt/provider/human_provider.py [36]

    -**kwargs
    +return self.ask(msg, timeout=self.get_timeout(timeout), **kwargs)
     

    @codecov-commenter
    Copy link

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 70.20%. Comparing base (f201b2f) to head (d4bd668).

    ❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

    Additional details and impacted files
    @@            Coverage Diff             @@
    ##             main    #1264      +/-   ##
    ==========================================
    - Coverage   70.22%   70.20%   -0.03%     
    ==========================================
      Files         316      316              
      Lines       18860    18860              
    ==========================================
    - Hits        13245    13240       -5     
    - Misses       5615     5620       +5     

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

    @geekan geekan requested a review from garylin2099 May 15, 2024 09:28
    @iorisa
    Copy link
    Collaborator

    iorisa commented May 16, 2024

    LGTM

    @iorisa iorisa merged commit 9ff1e2f into geekan:main May 16, 2024
    0 of 2 checks passed
    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

    3 participants