Provide a standard base class for creating custom Signature field type #8217
Conversation
dspy/adapters/types/base_type.py
Outdated
| # Custom type messages are only in user messages | ||
| continue | ||
|
|
||
| pattern = r"<<CUSTOM-TYPE-START-IDENTIFIER>>(.*?)<<CUSTOM-TYPE-END-IDENTIFIER>>" |
There was a problem hiding this comment.
Shall we make <CUSTOM-TYPE-START-IDENTIFIER> a const?
dspy/adapters/types/base_type.py
Outdated
| pattern = r"<<CUSTOM-TYPE-START-IDENTIFIER>>(.*?)<<CUSTOM-TYPE-END-IDENTIFIER>>" | ||
| result = [] | ||
| last_end = 0 | ||
| content = message["content"] |
There was a problem hiding this comment.
Is it a fair assumption that content here is always a single string rather than a content part list?
There was a problem hiding this comment.
With current adapter design, yes, we format user inputs into a string, then split around custom type. Before this PR, we only split around the image tag.
There was a problem hiding this comment.
Thanks, can we add a comment for the context?
There was a problem hiding this comment.
sg, added!
dspy/adapters/types/image.py
Outdated
| def format(self) -> Union[list[dict[str, Any]], str]: | ||
| try: | ||
| url = self.url | ||
| if isinstance(url, str) and "<DSPY_IMAGE_START>" in url and "<DSPY_IMAGE_END>" in url: |
There was a problem hiding this comment.
nit: should we make <DSPY_IMAGE_START> and <DSPY_IMAGE_END> a const?
There was a problem hiding this comment.
This is a condition check for legacy code's compatibility, which we can remove in 1-2 version, so i would prefer not exposing a file-level constant.
tests/adapters/test_chat_adapter.py
Outdated
| expected_image_content = {"type": "image_url", "image_url": {"url": "https://example.com/image.jpg"}} | ||
| assert expected_image_content in user_message_content | ||
|
|
||
| # Test formatting with few-shot examples |
There was a problem hiding this comment.
nit: shall we parameterize or split the test for clear test cases?
There was a problem hiding this comment.
Splitting sounds good to me
dspy/adapters/types/image.py
Outdated
| try: | ||
| url = self.url | ||
| if isinstance(url, str) and "<DSPY_IMAGE_START>" in url and "<DSPY_IMAGE_END>" in url: | ||
| url = url.split("<DSPY_IMAGE_START>", 1)[-1].split("<DSPY_IMAGE_END>", 1)[0] |
There was a problem hiding this comment.
nit: we can use re.search
There was a problem hiding this comment.
I read the existing dspy.Image's code again, and i think we can safely delete this code chunk.
There was a problem hiding this comment.
Yeah, I was just checking how DSPY_IMAGE_START is used after this code change and reached the same conclusion 😄
Modifying from #8210 by @arnavsinghvi11.
dspy.BaseTypewill be the base class for custom types for signature usage, includingdspy.Image,dspy.Audioand so on.dspy.Historydoes not subclass fromdspy.BaseTypebecause it needs to modify the signature and get formatted as multiturn messages.