Fix Regex DSL quantifier examples - #1964
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ErenAta16
left a comment
There was a problem hiding this comment.
Checked all three claims by running the doc's own snippets against main at be2cd15 rather than reading them. Every one holds.
The import fails:
>>> from outlines.types import String
ImportError: cannot import name 'String' from 'outlines.types'
>>> from outlines.types.dsl import String
>>>
All four functional quantifiers raise with the argument order the page currently shows:
exactly(Regex(r"\d"), 5) TypeError: '<' not supported between instances of 'Regex' and 'int'
at_least(Regex(r"\d"), 3) TypeError
at_most(Regex(r"\d"), 3) TypeError
between(Regex(r"\w"), 2, 4) TypeError
and the counts-first order in this PR works for all four.
Every documented output is wrong, and every replacement matches exactly what to_regex returns. Built each term the way the page writes it, including the method forms:
Regex(r"[0-9]+") actual ([0-9]+) page said [0-9]+
Regex(r"\d").exactly(5) actual ((\d)){5} page said (\d){5}
Regex(r"[A-Za-z]").one_or_more() actual (([A-Za-z]))+ page said ([A-Za-z])+
String(" ").zero_or_more() actual (\ )* page said ( )*
zero_or_more(" ") actual (\ )* page said ( )*
Seven of seven corrected values match on the nose, including the escaped space, which is the one I would have expected someone to normalise by hand and get wrong.
So this is a page where copying any quantifier example raises and every printed output is inaccurate. Worth merging.
One thing outside this PR's scope but worth writing down somewhere while it is fresh. Even after the page is correct, getting the order wrong gives you this:
TypeError: '<' not supported between instances of 'Regex' and 'int'
which does not tell you what you did. The count is being compared before anything checks it is a number, so the failure surfaces from inside the bounds check rather than at the boundary. A guard raising something like "expected an integer count, got Regex" would turn the most likely mistake with these functions into a message that names it. Happy to open that as a separate issue if you think it is worth having, since it is a code change rather than a docs one and does not belong in this PR.
The standalone quantifier examples currently pass the term before the count, while the public functions accept the count first. Copying those examples raises an error. The page also imports
Stringfrom a namespace that does not export it and shows output that differs fromto_regex.This updates the examples to use the public signatures, imports
Stringfromoutlines.types.dsl, and aligns the output comments with the values produced byto_regex.Verified with a script covering each documented expression and with:
Result: 61 passed.