docs(experiments): evaluation value cannot be None#1446
Merged
Conversation
Contributor
There was a problem hiding this comment.
Additional Comments (2)
-
langfuse/experiment.py, line 127 (link)logic: This example now violates the type hint on line 188, which no longer accepts
Noneas a valid value. The example should either handle the missing expected output differently or be removed.Or update to return a valid value instead of
None. -
langfuse/experiment.py, line 173 (link)logic: This example contradicts the updated type hint (line 188) which no longer allows
Nonevalues. Since evaluation values cannot beNone, this example should either return a sentinel value, skip the evaluation, or handle the error differently.Or use a different approach that doesn't use
value=None.
1 file reviewed, 2 comments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
Removes
Noneas a valid type forvalueinEvaluationclass inexperiment.py, updating the__init__method and class docstring accordingly.Noneas a valid type forvalueinEvaluationclass inexperiment.py.__init__method to enforcevalueasUnion[int, float, str, bool].Noneas a validvaluetype.This description was created by
for 306529c. You can customize this summary. It will automatically update as commits are pushed.
Disclaimer: Experimental PR review
Greptile Overview
Greptile Summary
This PR removes
Noneas a valid value forEvaluation.valueby updating the type hint fromUnion[int, float, str, bool, None]toUnion[int, float, str, bool]and removing the corresponding documentation line.Issue Found:
The PR incompletely updates the documentation. While the type hint and the main documentation list are updated, two code examples in the docstring (lines 127 and 173) still show
value=Nonebeing used in error handling scenarios. These examples now contradict the new type constraint and will cause type errors if users follow them.Impact:
Nonefrom the evaluator function to skip the evaluation, or using sentinel values)Confidence Score: 2/5
Important Files Changed
File Analysis
NonefromEvaluation.valuetype hint and docs, but examples still usevalue=None, creating contradictionsSequence Diagram
sequenceDiagram participant User participant Evaluator participant Evaluation participant LangfuseAPI User->>Evaluator: Call evaluator function Evaluator->>Evaluator: Process output alt Has expected_output Evaluator->>Evaluation: Create with value (int/float/str/bool) Note right of Evaluation: value cannot be None (after PR) else No expected_output Evaluator->>Evaluation: Previously: value=None Note right of Evaluation: Issue: Examples still use None<br/>but type hint now rejects it end Evaluation->>LangfuseAPI: Submit score LangfuseAPI-->>User: Result recorded