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 default to JSON schema when const is True #4152

Merged

Conversation

aminalaee
Copy link
Contributor

@aminalaee aminalaee commented Jun 11, 2022

Change Summary

Right now for the following Model:

from pydantic import BaseModel, Field

class MyModel(BaseModel):
    x: int = Field(default=0, const=True)

The JSON schema is:

{
  "title": "MyModel",
  "type": "object",
  "properties": {
    "x": {
      "title": "X",
      "const": 0,
      "type": "integer"
    }
  }
}

Which will include the default:

{
  "title": "MyModel",
  "type": "object",
  "properties": {
    "x": {
      "title": "X",
      "const": 0,
      "default": 0,
      "type": "integer"
    }
  }
}

I couldn't find the reason why it was introduced in #2094 but seems like the main purpose was something else.

Related issue number

Fixes #4031

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

@aminalaee
Copy link
Contributor Author

please review

@hramezani
Copy link
Member

I couldn't find the reason why it was introduced in #2094 but seems like the main purpose was something else.

The and not field.field_info.const was introduced originally in Implement const keyword in Schema and just reformatted in #2094. IMO, seems the original implementation was wrong.

@PrettyWood
Copy link
Member

PrettyWood commented Jul 1, 2022

IMO the const keyword should be deprecated in favor of Literal. With Literal, is the behaviour the expected one?
edit: I don't say we should deprecate it now or that this PR should not be merged! :P

@aminalaee
Copy link
Contributor Author

@PrettyWood

Not sure if I understand correctly but this will not include a default either:

class Test(BaseModel):
    x: Literal[10]
{"title": "Test", "type": "object", "properties": {"x": {"title": "X", "enum": [10], "type": "integer"}}, "required": ["x"]}

But this will which I'm not sure if is correct:

class Test(BaseModel):
    x: Literal[10] = Field(default=10)
{"title": "Test", "type": "object", "properties": {"x": {"title": "X", "default": 10, "enum": [10], "type": "integer"}}}

@aminalaee
Copy link
Contributor Author

And about the const deprecation, const=True will produce "const": 10 in schema but Literal[10] will produce "enum": [10] which should serve the same purpose but they are both possible according the the json schema, I could not find any references to the difference though.

@aminalaee aminalaee requested a review from hramezani July 5, 2022 18:16
@samuelcolvin
Copy link
Member

IMO the const keyword should be deprecated in favor of Literal. With Literal, is the behaviour the expected one? edit: I don't say we should deprecate it now or that this PR should not be merged! :P

I think what @PrettyWood means is that Literal[10] (just one option in the literal), should result in "const": 10 in the schema? I think he just means we should get rid of the kwarg from Field (at least that's what I think he means 🤨)

@samuelcolvin samuelcolvin merged commit 8f388e1 into pydantic:master Aug 8, 2022
@samuelcolvin
Copy link
Member

thanks so much for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add default in JSON Schema when const=True
4 participants