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(traces): server-side sort of spans by evaluation result (score or label) #1812

Merged
merged 6 commits into from
Nov 28, 2023

Conversation

RogerHYang
Copy link
Contributor

@RogerHYang RogerHYang commented Nov 28, 2023

resolves #1776

Screen.Recording.2023-11-27.at.4.09.17.PM.mov

Comment on lines +671 to +672
col: SpanColumn = null
evalResultKey: EvalResultKey = null
Copy link
Contributor Author

Choose a reason for hiding this comment

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

oneOf for input types is not available in strawberry

Comment on lines 49 to 68
def __call__(
self,
span: Span,
evals: Optional[SupportsGetSpanEvaluation] = None,
) -> Any:
"""
Returns the evaluation result for the given span
"""
if evals is None:
return None
span_id = span.context.span_id
evaluation = evals.get_span_evaluation(span_id, self.name)
if evaluation is None:
return None
result = evaluation.result
if self.attr is EvalAttr.score:
return result.score.value if result.HasField("score") else None
if self.attr is EvalAttr.label:
return result.label.value if result.HasField("label") else None
assert_never(self.attr)
Copy link
Contributor

Choose a reason for hiding this comment

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

minor nit on semantics: making the key a callable rather than having an explicit method that has a well-formed name is confusing for me. For example the code below doesn't reed well to me - the key appears to be a simple param but then it's getting partially called with evals. It forces me to have to go to definition to understand what calling the key does, which is a level of indirection that I think you can avoid by just having a good name for a method or function.

_key = partial(self.eval_result_key, evals=evals)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can change it. Not a problem. But will just note the reason why it was done.

  1. key is a callable in both python and pandas, so it's part of the python tradition.
  2. python functions are objects, so initializing it with parameters alludes to python's functional paradigm.
  3. it's fun since it's a programmer's version of a double entendre (given 1 and 2 above)

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, yeah I think Key being a callable makes sense but in this case it requires a partial application to work. But this makes sense. Your call. Thanks for the explanation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem. I already changed it.

@RogerHYang RogerHYang merged commit d139693 into main Nov 28, 2023
10 checks passed
@RogerHYang RogerHYang deleted the eval-sort branch November 28, 2023 21:56
mikeldking pushed a commit that referenced this pull request Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[traces] server-side sort on eval label and score
2 participants