Make schema work for Literal and NewType

[dmontagu]

Change Summary

BaseModel.schema() produces reasonable output for both Literal and NewType (both of which previously generated errors).

Related issue number

#646

Checklist

• Unit tests for the changes exist
• Tests pass on CI and coverage remains at 100%
• Documentation reflects the changes where applicable
• HISTORY.rst has been updated
  • if this is the first change since a release, please add a new section
  • include the issue number or this pull request number #<number>
  • include your github username @<whomever>

[dmontagu]

It might be preferable for this to look more like the schema for an Enum, but I'm not sure what is right; I'm not super knowledgeable about JSON schemas. Let me know if something else is preferred.

@layday I suppose you might have an opinion given you reported the issue?

[layday]

This should be enum:

Suggested change

	'c': {'anyOf': [{'type': 'integer'}, {'type': 'string'}], 'title': 'C'},
	'c': {'enum': ['a', 1], 'title': 'C'},

[dmontagu]

@samuelcolvin would you agree on this point? If so, I'll modify to ensure it gets 'enum' here

[layday]

1. 'anyOf' is wrong because in JSON Schema parlance 'anyOf' means it should validate against one or more subschemas in the array. You want to use 'oneOf'.
2. 'type' is wrong because it should match one of the values in the array exactly, meaning you'd have to use 'const'.
3. 'oneOf': [{'const': 'a'}, {'const': 1}] is a roundabout way to say 'enum': ['a', 1].

[samuelcolvin]

I'm not a JSONSchema expert either, @tiangolo what do you think?

[tiangolo]

I would suggest oneOf instead of enum.

That would allow having extra data in the schema. For example, if you use a NewType based on EmailStr, the type would be string with format: "email". But if it's an enum, it won't be able to have the extra format: "email".

Also, const can go along with type. type could be used by clients (e.g. frontends) to display the value in a certain way, even if it can only be a specific value.

E.g.:

'oneOf': [{'type': 'string', 'const': 'a'}, {'type': 'integer', 'const': 1}]

[layday]

Literals can only contain primitive literals - not NewType - so the situation you describe does not arise. But if you think including the type can be useful let's go with oneOf.

[dmontagu]

I have updated this to use 'const', but I refactored to use Union to handle multiple values. This is consistent with the semantics of Literal, (i.e., Literal[1, 2, 3] is the same as Union[Literal[1], Literal[2], Literal[3]]), and means that we can leverage the Union-handling logic to generate a multi-value schema.

Currently, this results in anyOf rather than oneOf. In the case of Literal, it is impossible to match on multiple, so anyOf is essentially equivalent to oneOf. I figured this was preferable to implementing custom handling of the multi-value case, but could rework to explicitly use oneOf if there is a good reason.

Alternatively, #656 might result in Union using oneOf anyway, in which case the point would be moot anyway.

[tiangolo]

Great! Agree.

And actually, after thinking about it, I guess anyOf makes more sense, as I guess there might be cases where even with Literal, a single value could be valid with more than one schema. Not sure, but in any case, it is not enforcing that a value not valid against the other schemas, so, it's not really enforcing a oneOf.

[tiangolo]

In short, agreed.

[codecov]

Codecov Report

Merging #649 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #649   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          15     15           
  Lines        2651   2671   +20     
  Branches      524    529    +5     
=====================================
+ Hits         2651   2671   +20

[dmontagu]

@samuelcolvin I think the discussion of the proper schema to use for Literal has been resolved.

I'm keen to get this merged because, while I have a workaround I'm happy with for now for non-UUID types, the workaround for uuid.UUID-based NewTypes is currently a little more painful (I describe this in more detail below).

If it would make it easier for you to review, I would be happy to split this into two pull requests (one for NewType and one for Literal). I would just like to get the NewType fix into a release as soon as possible, as it will allow me to remove a decent amount of cruft from my code.

---

For most types involved with NewType (e.g., str, int, etc.), type(type(x)) == type(x). Because of this, I can use the following work around:

if TYPE_CHECKING:
    MyLabel = NewType("MyLabel", str)
else:
    MyLabel = str

This way, type checking works properly, and if I use the "NewType" as a field label, it is interpreted correctly by pydantic. This results in a little more runtime overhead than NewType, but at least everything works properly.

Unfortunately, uuid.UUID(uuid.UUID('00000000-0000-0000-0000-000000000000')) raises an exception. As a result, if I try to cast a UUID to a specific NewType for type hinting, it causes runtime errors. In order to deal with this, I need to create an assignment-type-ignored local variable every time I want to use one:

if TYPE_CHECKING:
    MyID = NewType("MyID", uuid.UUID)
else:
    MyID = uuid.UUID

random_id: MyID = uuid.UUID(uuid4_str())  # type: ignore

However, needing to create a local variable for this every time is starting to pollute my code, and worse, mypy can't notify me if I forget to avoid casting uuids to the typed variant (leading to annoying runtime errors).

[samuelcolvin]

I'm happy with this.

Sorry I've been absent in this conversation, I was at pycon europe, then at a friends wedding the sorting the rest of my life out.

I'll try to put some time into pydantic later in the week to get this and other PRs merged and released and work out what to do about v1.

[samuelcolvin]

@dmontagu please could you rebase and update HISTORY, as per #664 I've just made a release so you'll need a new section.

[dmontagu]

@samuelcolvin No worries! Thanks for being so consistently responsive. The history should be fixed.