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

Make sure we correctly support additionalProperties set to True #184

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yvan-sraka
Copy link
Contributor

This PR is a quick and dirty fix for #150. It makes the example highlighted in the issue work as expected but breaks other tests doing so...

import json
import re
from outlines_core.json_schema import build_regex_from_schema
schema = {
    "type": "object",
    "properties": {
        "a": {
            "type": "string",
        }
    },
    "additionalProperties": True
}
regex_str = build_regex_from_schema(json.dumps(schema))
data = json.dumps({
    "a": "aaa",
    "id": "1"
})
print(re.match(regex_str, data))

The overall design of JSON Schema parsing here is fragile and difficult to extend without a full refactoring... So, I've added a few comments to highlight the problem and suggest what could be a better implementation!

@yvan-sraka yvan-sraka requested a review from torymur February 26, 2025 15:06
@yvan-sraka yvan-sraka self-assigned this Feb 26, 2025
@yvan-sraka yvan-sraka force-pushed the 150-make-sure-we-correctly-support-additionalproperties-set-to-true branch from cb81ab3 to b15b43f Compare February 26, 2025 16:58
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.

1 participant