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

Update chat template docs and bump Jinja version #31455

Merged
merged 10 commits into from
Jun 18, 2024

Conversation

Rocketknight1
Copy link
Member

This PR makes some updates to the chat template docs. Specifically, I put much more emphasis on using trimming blocks like {{- and {%- to let you write neater, indented code, and I also add a section on making your template compatible with non-Python implementations of Jinja. This PR also bumps the minimum Jinja version to 3.1, which is the earliest version that supports the filters we recommend in these docs (and have started to use in our templates), such as |items.

Copy link
Collaborator

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for adding!

@@ -441,8 +441,8 @@ def compile_jinja_template(template):
except ImportError:
raise ImportError("template requires jinja2 to be installed.")

if version.parse(jinja2.__version__) <= version.parse("3.0.0"):
raise ImportError("template requires jinja2>=3.0.0 to be installed. Your version is " f"{jinja2.__version__}.")
if version.parse(jinja2.__version__) < version.parse("3.1.0"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this match with jinja specified in the library e.g. setup.py?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't actually make Jinja a library requirement - chat templates were a somewhat peripheral feature, so I didn't want to add an forced dependency for them at first, plus they're already a dependency for torch.

However, given that they're becoming a lot more core, maybe I should consider doing that in a separate PR!

Copy link
Contributor

@xenova xenova left a comment

Choose a reason for hiding this comment

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

LGTM - just some minor nits! 🚢

docs/source/en/chat_templating.md Outdated Show resolved Hide resolved
docs/source/en/chat_templating.md Outdated Show resolved Hide resolved
docs/source/en/chat_templating.md Outdated Show resolved Hide resolved

By default, Jinja will print any whitespace that comes before or after a block. This can be a problem for chat
templates, which generally want to be very precise with whitespace! To avoid this, we strongly recommend writing
your templates with `{{-` and `{%-` blocks instead of `{{` and `{%`. This will strip any whitespace that comes
Copy link
Contributor

Choose a reason for hiding this comment

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

Mentioned above in the Tip already, but not opposed to having it mentioned again!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, I know it went in twice - at least that one was deliberate!

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@Rocketknight1 Rocketknight1 force-pushed the chat_template_doc_update branch from 693e77e to ee919ec Compare June 18, 2024 12:54
@Rocketknight1 Rocketknight1 merged commit 6e56b83 into main Jun 18, 2024
23 checks passed
@Rocketknight1 Rocketknight1 deleted the chat_template_doc_update branch June 18, 2024 13:16
itazap pushed a commit that referenced this pull request Jun 18, 2024
* Update chat template docs

* Minor bug in the version check

* Update docs/source/en/chat_templating.md

Co-authored-by: Joshua Lochner <admin@xenova.com>

* Update docs/source/en/chat_templating.md

Co-authored-by: Joshua Lochner <admin@xenova.com>

* Update docs/source/en/chat_templating.md

Co-authored-by: Joshua Lochner <admin@xenova.com>

* Replace backticks with bolding because the doc builder was trying to parse them

* Replace backticks with bolding because the doc builder was trying to parse them

* Replace backticks with bolding because the doc builder was trying to parse them

* More cleanups to avoid upsetting the doc builder

* Add one more tip at the end

---------

Co-authored-by: Joshua Lochner <admin@xenova.com>
itazap pushed a commit that referenced this pull request Jun 20, 2024
* Update chat template docs

* Minor bug in the version check

* Update docs/source/en/chat_templating.md

Co-authored-by: Joshua Lochner <admin@xenova.com>

* Update docs/source/en/chat_templating.md

Co-authored-by: Joshua Lochner <admin@xenova.com>

* Update docs/source/en/chat_templating.md

Co-authored-by: Joshua Lochner <admin@xenova.com>

* Replace backticks with bolding because the doc builder was trying to parse them

* Replace backticks with bolding because the doc builder was trying to parse them

* Replace backticks with bolding because the doc builder was trying to parse them

* More cleanups to avoid upsetting the doc builder

* Add one more tip at the end

---------

Co-authored-by: Joshua Lochner <admin@xenova.com>
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.

4 participants