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: validate template variables before apply and improve sliding wi… #2403

Merged
merged 2 commits into from
Aug 12, 2024

Conversation

drbh
Copy link
Collaborator

@drbh drbh commented Aug 12, 2024

This PR improves the template to throw a better error the guideline is included in a template, but not provided in the request

Copy link
Collaborator

@Narsil Narsil left a comment

Choose a reason for hiding this comment

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

LGTM !

Comment on lines +74 to +76
if self.variables.contains("guideline") && guideline.is_none() {
return Err(InferError::MissingTemplateVariable("guideline".to_string()));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if there's a way to make it generic somehow ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yea I was thinking of a generic solution, but I think those changes would also require an update to the apply signature (to be more generic) so maybe those can be improved together in the future (as more variables are needed)

@drbh drbh merged commit 155f9c9 into main Aug 12, 2024
10 checks passed
@drbh drbh deleted the validate-template-variables-before-apply branch August 12, 2024 14:58
yuanwu2017 pushed a commit to yuanwu2017/tgi-gaudi that referenced this pull request Sep 26, 2024
huggingface#2403)

* feat: validate template variables before apply and improve sliding window check

* fix: improve missing template var test
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