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

Fix support for Python template syntax #429

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

jeeger
Copy link

@jeeger jeeger commented Feb 28, 2024

All dollar signs in a template's prompt or system values are interpreted as introducing placeholders. The documentation for string.Template says that doubling a dollar sign should escape it.

Due to the custom implementation of Template.extract_vars, this did not work, as it extracted a double dollar sign as an identifier with an empty named group. By using string.Template.get_identifiers() instead, this now works. Additionally, this automatically adds support for parenthesized identifiers as well.

…or escaped identifiers.

The previous implementation failed with doubled dollar signs, which, according to the Python
template syntax, should be used to escape a single dollar sign. Additionally added a test for
a template containing an escaped dollar sign.
Added a version check that uses `string.Template.get_identifiers` if Python is > 3.11, falls back to
a reimplementation.
@simonw
Copy link
Owner

simonw commented Mar 9, 2024

Tests failed on Python 3.8:

llm/templates.py:54: in Template
    def extract_identifiers(cls, template: string.Template) -> list[str]:
E   TypeError: 'type' object is not subscriptable

I'll fix that now.

@jeeger
Copy link
Author

jeeger commented Mar 18, 2024

Sorry for the bad code, I should figure out how to run the tests locally.

@jeeger
Copy link
Author

jeeger commented Mar 18, 2024

There, that should do it. Sorry for the noise, I was apparently coding on half a brain last week.

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