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

[ML-OB] document submit_evaluation_for API in the Python SDK #27075

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

Conversation

lievan
Copy link
Contributor

@lievan lievan commented Jan 13, 2025

What does this PR do? What is the motivation?

We made an update to the Eval metric API to allow tag-based joining for eval metrics (#27074)

This PR documents the corresponding SDK method that supports tag-based joining

Merge instructions

Merge readiness:

  • Ready for merge

Merge queue is enabled in this repo. To have it automatically merged after it receives the required reviews, create the PR (from a branch that follows the <yourname>/description naming convention) and then add the following PR comment:

/merge

Additional notes

@lievan lievan requested a review from a team as a code owner January 13, 2025 15:12
Copy link
Contributor

github-actions bot commented Jan 13, 2025

@lievan lievan changed the title [ML-OB] document submit_evaluation_for API in the Python SDK [ML-OB] document submit_evaluation_for API in the Python SDK Jan 13, 2025
Copy link
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

Choose a reason for hiding this comment

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

a couple thoughts, otherwise looks good to me

@@ -519,45 +519,21 @@ def rag_workflow(user_question):

## Evaluations

The LLM Observability SDK provides the methods `LLMObs.export_span()` and `LLMObs.submit_evaluation()` to help your traced LLM application submit evaluations to LLM Observability.
Copy link
Member

Choose a reason for hiding this comment

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

we probably wanna keep a reference to the old function and mention that it's deprecated so that if someone tries to go back to our docs to find it they aren't too confused.

content/en/llm_observability/setup/sdk/python.md Outdated Show resolved Hide resolved
Copy link
Contributor

@Yun-Kim Yun-Kim left a comment

Choose a reason for hiding this comment

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

A couple style/format suggestions

content/en/llm_observability/setup/sdk/python.md Outdated Show resolved Hide resolved
content/en/llm_observability/setup/sdk/python.md Outdated Show resolved Hide resolved
content/en/llm_observability/setup/sdk/python.md Outdated Show resolved Hide resolved
content/en/llm_observability/setup/sdk/python.md Outdated Show resolved Hide resolved
content/en/llm_observability/setup/sdk/python.md Outdated Show resolved Hide resolved
content/en/llm_observability/setup/sdk/python.md Outdated Show resolved Hide resolved
content/en/llm_observability/setup/sdk/python.md Outdated Show resolved Hide resolved
content/en/llm_observability/setup/sdk/python.md Outdated Show resolved Hide resolved
@rtrieu rtrieu added the editorial review Waiting on a more in-depth review label Jan 13, 2025
@rtrieu
Copy link
Contributor

rtrieu commented Jan 13, 2025

Created DOCS-9870 for docs review.

Copy link
Contributor

@Yun-Kim Yun-Kim left a comment

Choose a reason for hiding this comment

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

LGTM other than one small suggestion

Copy link
Contributor

@joepeeples joepeeples left a comment

Choose a reason for hiding this comment

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

Noted a couple of typos. Just let me know when this is ready for quick approval, thanks!

content/en/llm_observability/submit_evaluations.md Outdated Show resolved Hide resolved
content/en/llm_observability/setup/sdk/python.md Outdated Show resolved Hide resolved
content/en/llm_observability/setup/sdk/python.md Outdated Show resolved Hide resolved
content/en/llm_observability/setup/sdk/python.md Outdated Show resolved Hide resolved
@lievan lievan requested a review from joepeeples January 17, 2025 14:48
@lievan lievan added the Do Not Merge Just do not merge this PR :) label Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Do Not Merge Just do not merge this PR :) editorial review Waiting on a more in-depth review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants