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

Add from_file class method to the Prompt object #1355

Merged
merged 3 commits into from
Jan 9, 2025

Conversation

yvan-sraka
Copy link
Contributor

@yvan-sraka yvan-sraka commented Jan 2, 2025

Fix #1345: For now, I'm just trying to make sure I'm editing the right part of the codebase. I haven't managed to run the tests yet…

EDIT: I opened a separate issue about the falling tests 🙂

I noticed that the project use pytest, so I assumed that running pip install ".[test]" would install all the dependencies needed to run the full test suite. However, it seems that's not the case (it's currently complaining that I don't have vllm). Should I open an issue (I can add the missing dependency under the test section of the pyproject.toml)?

#1356

@yvan-sraka yvan-sraka requested review from torymur and rlouf January 2, 2025 13:54
@yvan-sraka yvan-sraka self-assigned this Jan 2, 2025
@yvan-sraka yvan-sraka linked an issue Jan 2, 2025 that may be closed by this pull request
@yvan-sraka yvan-sraka force-pushed the 1345-add-from_file-class-method-to-the-prompt-object branch 3 times, most recently from b2605b3 to df0eb2c Compare January 2, 2025 14:36
@rlouf
Copy link
Member

rlouf commented Jan 2, 2025

Yes, please open an issue 😄

@yvan-sraka yvan-sraka force-pushed the 1345-add-from_file-class-method-to-the-prompt-object branch from f11f10c to 98cef53 Compare January 2, 2025 17:01
outlines/prompts.py Outdated Show resolved Hide resolved
tests/test_prompts.py Outdated Show resolved Hide resolved
@rlouf
Copy link
Member

rlouf commented Jan 2, 2025

Added a couple comments, although since this is a draft PR you are probably addressing them already.

@yvan-sraka yvan-sraka force-pushed the 1345-add-from_file-class-method-to-the-prompt-object branch 6 times, most recently from f75319f to 2954197 Compare January 6, 2025 09:51
@yvan-sraka yvan-sraka requested a review from rlouf January 6, 2025 10:00
@yvan-sraka yvan-sraka marked this pull request as ready for review January 6, 2025 10:00
@yvan-sraka yvan-sraka force-pushed the 1345-add-from_file-class-method-to-the-prompt-object branch from 2954197 to ee5c4ef Compare January 6, 2025 10:45
@yvan-sraka
Copy link
Contributor Author

Working on this PR made me wonder: why do we integrate Jinja2 (or any specific templating engine) into Outlines? I mean, we could have just provided a Prompt interface that takes an already rendered str (and eventually keep a function decorator if that sounds fancy and helps better infer the template arguments), leaving the templating up to the user. That way, users could choose a different templating engine if they wanted, and e.g., how to use Outlines in combination with Jinja2 could be rather provided as an example in the documentation. WDYT?

@rlouf
Copy link
Member

rlouf commented Jan 6, 2025

I am not sure what you mean by "taking an already rendered str", could you provide an example?

@yvan-sraka
Copy link
Contributor Author

yvan-sraka commented Jan 6, 2025

I am not sure what you mean by "taking an already rendered str", could you provide an example?

TL;DR: I mean passing the output/result of the templating process to the Prompt constructor, thereby externalizing the templating from Outlines. IMHO, this would lead to better separation of concerns and make Outlines lighter and simpler. One idea could be to maintain tighter integration with templating only as an optional feature, if we believe it simplifies the workflow and improves the developer experience.

import outlines

# Using Jinja2 ...
import jinja2

env = jinja2.Environment(loader=jinja2.FileSystemLoader("."))
template = env.get_template("prompt.jinja2")

# ... or, Using Mako ...
import Mako

template = mako.template.Template(filename="prompt.mako")

@outlines.prompt
def fancy(examples, question):
    return template.render(examples=examples, question=question)

examples = [
    {"question": "What is the capital of France?", "answer": "Paris"},
    {"question": "What is 2 + 2?", "answer": "4"},
]
question = "What is the Earth's diameter?"

prompt = fancy(examples, question)

# ... but why not just?
rendered = template.render(examples=examples, question=question)
prompt = outline.Prompt(rendered)
# EDIT: Nevermind, while writing this down, I realized that this would
# essentially mean that arguments are evaluated too early, leaving the prompt
# without any arguments and simply storing a `str`...

outlines/prompts.py Outdated Show resolved Hide resolved
outlines/prompts.py Outdated Show resolved Hide resolved
@rlouf
Copy link
Member

rlouf commented Jan 6, 2025

You can already use any templating engine you want with Outlines, by rendering the template and passing the returned string to a generator:

import outlines

model = outlines.models.openai("gpt-4o")

# Input

examples = [
    {"question": "What is the capital of France?", "answer": "Paris"},
    {"question": "What is 2 + 2?", "answer": "4"},
]
question = "What is the Earth's diameter?"


# Using Jinja2 ...
import jinja2

env = jinja2.Environment(loader=jinja2.FileSystemLoader("."))
template = env.get_template("prompt.jinja2")
rendered = template.render(examples=examples, question=question)

generator = outlines.generate.text(model)
results = generator(rendered)


# ... or, Using Mako ...
import Mako

template = mako.template.Template(filename="prompt.mako")
rendered = template.render(examples=examples, question=question)

generator = outlines.generate.text(model)
results = generator(rendered)

The role of the Prompt object is to wrap a Jinja2 template so that it returns a string when called with the template's variable values. I chose Jinja2 to make the common path easier; users of outlines are not necessarily familiar with templating engines and Jinja2 seemed like a reasonable default. As I demonstrated above you can always use another templating engine, at the cost of extra complexity.

The confusion may come from poor naming on my part; this object should probably be called Template instead.

@yvan-sraka
Copy link
Contributor Author

yvan-sraka commented Jan 6, 2025

Thank you very much @rlouf for the clarification. I'm not sure about the naming either, but if it's extensible, that already sounds like a great design! Apologies for sharing ideas maybe a bit too quickly as they pop from my mind, I probably should have checked the documentation first!

@yvan-sraka yvan-sraka force-pushed the 1345-add-from_file-class-method-to-the-prompt-object branch 2 times, most recently from 5aad80d to d318ac2 Compare January 7, 2025 14:35
@yvan-sraka yvan-sraka marked this pull request as draft January 7, 2025 14:38
@yvan-sraka
Copy link
Contributor Author

(Do not merge this PR yet, I will rewrite the commit history a bit once we've settled on deprecating or removing outlines.prompts.render!)

@yvan-sraka yvan-sraka force-pushed the 1345-add-from_file-class-method-to-the-prompt-object branch 5 times, most recently from 2b87ecb to 32587ee Compare January 7, 2025 16:01
@yvan-sraka
Copy link
Contributor Author

Help! I don't understand the failed test coverage. It says that only line 90 of prompt.py is not covered, which doesn't make sense to me…

@torymur
Copy link
Contributor

torymur commented Jan 8, 2025

It seems that tests don't cover that part of the flow, where if is True and cleaned_template += "\n" is actually being executed:

        ends_with_linebreak = content.replace(" ", "").endswith("\n\n")
        if ends_with_linebreak:
            cleaned_template += "\n"

This is one of the cases, I suppose, where one of the old render tests would exactly cover (I would integrate them all into template_from_str):

    tpl = """
        A test line
            An indented line

    """
    assert render(tpl) == "A test line\n    An indented line\n"

@yvan-sraka yvan-sraka force-pushed the 1345-add-from_file-class-method-to-the-prompt-object branch 5 times, most recently from 9f1dca8 to 38d920f Compare January 9, 2025 11:09
Copy link
Contributor

@torymur torymur left a comment

Choose a reason for hiding this comment

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

Turned out nicely, looking good! 🚀

tests/test_prompts.py Outdated Show resolved Hide resolved
@yvan-sraka yvan-sraka force-pushed the 1345-add-from_file-class-method-to-the-prompt-object branch from 38d920f to 6286db7 Compare January 9, 2025 16:06
@yvan-sraka yvan-sraka marked this pull request as ready for review January 9, 2025 16:07
@yvan-sraka yvan-sraka force-pushed the 1345-add-from_file-class-method-to-the-prompt-object branch from 6286db7 to 066912f Compare January 9, 2025 16:18
@yvan-sraka
Copy link
Contributor Author

(Do not merge this PR yet, I will rewrite the commit history a bit once we've settled on deprecating or removing outlines.prompts.render!)

I've cleaned up the git history, so it's ready to be merged now :)

@rlouf
Copy link
Member

rlouf commented Jan 9, 2025

Great job! 🎉

@rlouf rlouf merged commit 7149a5e into main Jan 9, 2025
7 checks passed
@rlouf rlouf deleted the 1345-add-from_file-class-method-to-the-prompt-object branch January 9, 2025 18:59
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.

Add from_file class method to the Prompt object
3 participants