Skip to content

Conversation

@PLAYPANDA
Copy link
Contributor

@PLAYPANDA PLAYPANDA commented Apr 3, 2025

Refactor evaluate function to return an awaitable handle using asyncio. This change addresses the issue where the event loop was already running, preventing proper execution of the evaluation function.
#101


Important

Refactor evaluate() in evaluations.py to return an awaitable handle when the event loop is already running, addressing issue #101.

  • Behavior:
  • Misc:
    • No changes to function signatures or parameters.

This description was created by Ellipsis for a1d180e. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to a1d180e in 51 seconds

More details
  • Looked at 12 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. src/lmnr/sdk/evaluations.py:484
  • Draft comment:
    When the event loop is running, the function now returns an awaitable (evaluation.run()) instead of awaiting it. Ensure that the caller properly awaits this returned coroutine to avoid unexpected behavior.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to ensure that the caller properly awaits the returned coroutine. This falls under the category of asking the author to ensure behavior is intended or tested, which violates the rules.
2. src/lmnr/sdk/evaluations.py:483
  • Draft comment:
    Returning evaluation.run() directly when the loop is already running is the correct fix for the running event loop. Ensure callers await the returned coroutine.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
3. src/lmnr/sdk/evaluations.py:115
  • Draft comment:
    Typo: The init docstring says 'Initializes an instance of the Evaluations class.' Consider changing 'Evaluations' to 'Evaluation' to match the class name.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. src/lmnr/sdk/evaluations.py:422
  • Draft comment:
    Typo/Redundancy: The 'evaluators' parameter in the evaluate() function docstring is documented twice with different type hints. Consider removing or merging the duplicate descriptions for clarity.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_ySCwLGs2KkO29Kve


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@dinmukhamedm
Copy link
Member

Thanks for this @PLAYPANDA

@dinmukhamedm dinmukhamedm merged commit a1b9c5b into lmnr-ai:main Apr 4, 2025
7 checks passed
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.

2 participants