Adds modular SERDE approach #175
Draft
Conversation
This file contains 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
LC documents can now be serialized properly. Note: LC produces things that can look like documents that aren't. E.g. _DocumentWithState Should add tests here eventually.
skrawcz
added a commit
to skrawcz/Scrapegraph-ai
that referenced
this pull request
May 11, 2024
skrawcz
added a commit
to skrawcz/Scrapegraph-ai
that referenced
this pull request
May 11, 2024
| @@ -20,9 +20,13 @@ | |||
|
|
|||
| try: | |||
There was a problem hiding this comment.
So, we'll want to put this in a plugin and have a registry. Specifically for default serializers. But this OK as a temporary fix -- the "common" models aren't throw-away code, so let's do the following:
- Comment on TODO
- Open an issue on ser/deser (if we don't have one already) + plugins and document this out
- Link to that issue
I know we already have one in this, but having anything referring lanchain in the core code feels icky, even though its in a guarded setting.
| if isinstance(d, list): | ||
| return [_serialize_object(x) for x in d] | ||
| elif isinstance(d, dict): | ||
| return {k: _serialize_object(v) for k, v in d.items()} | ||
| elif lc_messages is not None and isinstance(d, lc_messages.BaseMessage): | ||
| return lc_messages.message_to_dict(d) | ||
| elif lc_documents is not None and isinstance(d, lc_documents.Document): |
There was a problem hiding this comment.
Ditto with the above. Blech custom serde is ugly...
Singledispatch for type serialization. For non-primitive types, make them return a dict with a serde key. Then use a string value based mechanism to deserialize. This way we can be "field agnostic" during deserialization. Otherwise deserialization without knowing what it is, is a challenge. Thus the idea to include a key in a dict for it. Otherwise the only other way for deserialization is to field registries... TODO: - determine module naming and location - actually wire in - proper tests - how to override these - think about any other edge/error cases
skrawcz
changed the title
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.
This PR introduces a modular SERDE approach.
of a "string dispatcher function". i.e. like single dispatch, but you register a string value for it. This approach
means that most people will get around the need for field level deserialization / having to tell Burr about state types explicitly.
TODOs:
Changes
How I tested this
Notes
Checklist