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: support serializing/deserializing baml_py.Image, baml_py.Audio with pydantic #1062

Merged
merged 7 commits into from
Oct 21, 2024

Conversation

sxlijin
Copy link
Collaborator

@sxlijin sxlijin commented Oct 19, 2024

Important

Add Pydantic support for baml_py.Image and baml_py.Audio serialization/deserialization with tests and documentation updates.

  • Behavior:
    • Add Pydantic serialization/deserialization for baml_py.Image and baml_py.Audio in media_repr.rs, audio.rs, and image.rs.
    • Implement baml_serialize and baml_deserialize methods for BamlImagePy and BamlAudioPy.
  • Documentation:
    • Update supported-types.mdx with Pydantic compatibility details for Image.
  • Testing:
    • Add test_pydantic.py to validate Pydantic model creation and serialization for baml_py.Image.
  • Misc:
    • Add Rust lints for dead_code and unused_imports in Cargo.toml.

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

Copy link

vercel bot commented Oct 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
baml ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 21, 2024 7:21pm

Copy link

@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 9d6fe0c in 29 seconds

More details
  • Looked at 496 lines of code in 10 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_Z4acSdfijKOd3vqI


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

Copy link

@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! Incremental review on ddb0300 in 12 seconds

More details
  • Looked at 33 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. engine/language_client_python/src/errors.rs:65
  • Draft comment:
    Avoid unnecessary clone() calls on prompt, message, and raw_output if they are already owned values. This can help reduce performance overhead.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of clone() on prompt, message, and raw_output in the raise_baml_validation_error call is unnecessary if these are already owned values. This can lead to unnecessary performance overhead.

Workflow ID: wflow_RaaQjW68QV2Eldep


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

Copy link

@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! Incremental review on 5f5f3c4 in 13 seconds

More details
  • Looked at 17 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. engine/language_client_python/src/types/media_repr.rs:71
  • Draft comment:
    Consider moving the explanation about not placing this function in baml_py.internal_monkeypatch to documentation or a design document for better maintainability.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The comment block from lines 71 to 75 explains why the function is not placed in baml_py.internal_monkeypatch. This is a good explanation, but it might be more appropriate to place this explanation in the documentation or a design document rather than in the code comments.

Workflow ID: wflow_HhsWj9APiypDz5bA


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

Copy link

@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! Incremental review on 58a44f8 in 22 seconds

More details
  • Looked at 37 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. engine/language_client_python/src/types/function_results.rs:3
  • Draft comment:
    Remove unused import PyListMethods.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_su0uEn4lCNocEIDW


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

@sxlijin sxlijin added this pull request to the merge queue Oct 21, 2024
Merged via the queue into canary with commit 11cb699 Oct 21, 2024
9 of 10 checks passed
@sxlijin sxlijin deleted the sam/pydantic-image branch October 21, 2024 19:20
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