feat: allow specifying the chat template as base64 to avoid weird escaping and templating issues#534
Conversation
|
Thanks for making a pull request! 😃 |
fa7d873 to
87716f0
Compare
dushyantbehl
left a comment
There was a problem hiding this comment.
@HarikrishnanBalagopal Please add a unit test for the base64 chat template and also say somewhere that this is a undocumented feature...i.e. in the schema you should try to add chat_template_base64 but add a note that this is something users should use if they know what they are doing.
Any way to check if the base64 decode happened correctly? or throw some error if it didn't result in a chat template?
| assert isinstance( | ||
| chat_template_base64, str | ||
| ), "chat_template_base64 should be a string" | ||
| logger.warning( |
There was a problem hiding this comment.
you should also look to print the chat tempalte along side the warning...what was decoded one so users know what was picked up.
There was a problem hiding this comment.
...what was decoded one so users know what was picked up.
ok but if it's not a string then we can't decode it right?
There was a problem hiding this comment.
you can add that post the warning that you have picked this chat template at that place it will be a str due to assertion
Then it would become documented, wouldn't it?
The The |
eb493b5 to
2e7d5f8
Compare
I have added tests |
132c9de to
eec61df
Compare
dushyantbehl
left a comment
There was a problem hiding this comment.
Minor changes requested
| ) | ||
| DATA_CONFIG_MULTITURN_CHAT_TOKENIZE_AND_MASKING_DATA_BASE64_HANDLER_EXPECTED_CHAT_TEMPLATE = os.path.join( | ||
| PREDEFINED_DATA_CONFIGS, | ||
| "mt_data_granite_3_1B_tokenize_and_mask_base64_handler_expected_chat_template.txt", |
There was a problem hiding this comment.
rename this file also to
granite_3_1b_chat_template.txt
| assert isinstance( | ||
| chat_template_base64, str | ||
| ), "chat_template_base64 should be a string" | ||
| logger.warning( |
There was a problem hiding this comment.
you can add that post the warning that you have picked this chat template at that place it will be a str due to assertion
|
@HarikrishnanBalagopal requested minor name changes and post them please also fix the format error |
21777b4 to
8075828
Compare
done |
…aping and templating issues test: create tests for chat_template_base64 field Signed-off-by: Harikrishnan Balagopal <harikrishmenon@gmail.com>
8075828 to
4289e4c
Compare
…aping and templating issues (foundation-model-stack#534) test: create tests for chat_template_base64 field Signed-off-by: Harikrishnan Balagopal <harikrishmenon@gmail.com>
Description of the change
Related issue number
How to verify the PR
Was the PR tested