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

add upset examples multipart endpoint #1209

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

Conversation

isahers1
Copy link
Contributor

No description provided.

python/langsmith/schemas.py Outdated Show resolved Hide resolved
Comment on lines 3442 to 3446
if encoder.len <= 20_000_000: # ~20 MB
data = encoder.to_string()
else:
data = encoder

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming this is what we do for runs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah this code was copy pasted

python/langsmith/client.py Outdated Show resolved Hide resolved
python/langsmith/client.py Outdated Show resolved Hide resolved
def upsert_examples_multipart(
self,
*,
upserts: List[ls_schemas.ExampleCreateWithAttachments] = [],
Copy link
Contributor

Choose a reason for hiding this comment

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

should not initialize this to empty list due to the way python handles mutable default arguments, see here https://nikos7am.com/posts/mutable-default-arguments/

to fix, just remove the default

Comment on lines 3380 to 3384
if not isinstance(upserts, list):
raise TypeError(f"upserts must be a list, got {type(upserts)}")
for item in upserts:
if not isinstance(item, ls_schemas.ExampleCreateWithAttachments):
raise TypeError(f"Each item must be ExampleCreateWithAttachments, got {type(item)}")
Copy link
Contributor

Choose a reason for hiding this comment

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

not necessary, we don't check types like this elsewhere

python/langsmith/client.py Outdated Show resolved Hide resolved
python/langsmith/client.py Outdated Show resolved Hide resolved
python/langsmith/client.py Outdated Show resolved Hide resolved
python/langsmith/client.py Outdated Show resolved Hide resolved
python/langsmith/client.py Outdated Show resolved Hide resolved
python/langsmith/client.py Show resolved Hide resolved
python/langsmith/client.py Outdated Show resolved Hide resolved
python/bench/upload_examples_bench.py Outdated Show resolved Hide resolved
python/bench/upload_examples_bench.py Outdated Show resolved Hide resolved
python/bench/upload_examples_bench.py Outdated Show resolved Hide resolved
python/langsmith/client.py Outdated Show resolved Hide resolved
python/bench/upload_examples_bench.py Show resolved Hide resolved
python/langsmith/client.py Outdated Show resolved Hide resolved
)

with pytest.raises(LangSmithNotFoundError):
langchain_client.upsert_examples_multipart(upserts=[example_3])
Copy link
Contributor

Choose a reason for hiding this comment

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

why should this raise a not found?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because it can't find the dataset I believe

@isahers1 isahers1 changed the title wip add upset examples multipart endpoint Nov 15, 2024
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