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

Possible regression of __modify_schema__ on master #1552

Closed
therefromhere opened this issue May 25, 2020 · 0 comments · Fixed by #1562
Closed

Possible regression of __modify_schema__ on master #1552

therefromhere opened this issue May 25, 2020 · 0 comments · Fixed by #1562
Labels
bug V1 Bug related to Pydantic V1.X

Comments

@therefromhere
Copy link
Contributor

Bug

I think #1422 causes an unintentional (?) change in the behaviour of __modify_schema__ - it's now applying to the list schema as well as the item schema.

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

             pydantic version: 1.5.1
            pydantic compiled: False
                 install path: /home/johnc/Projects/pydantic/pydantic
               python version: 3.7.7 (default, Apr 18 2020, 02:59:53)  [GCC 9.3.0]
                     platform: Linux-5.4.0-29-generic-x86_64-with-Ubuntu-20.04-focal
     optional deps. installed: []

Actually at git version 5195e55 -

pip freeze | grep pydantic
-e git+https://github.com/samuelcolvin/pydantic.git@5195e55c102e537be6c00579df8f464ecd90b5ff#egg=pydantic

The output of the following code is different between master and v1.5.1

from typing import List

import pydantic


class MyField(str):
  @classmethod
  def __modify_schema__(cls, field_schema):
    field_schema["foo"] = "buzz"


class Foo(pydantic.BaseModel):
  a_field: MyField

  some_fields: List[MyField]

print(Foo.schema_json(indent=2))

With 1.5.1 this outputs:

{
  "title": "Foo",
  "type": "object",
  "properties": {
    "a_field": {
      "title": "A Field",
      "type": "string",
      "foo": "buzz"
    },
    "some_fields": {
      "title": "Some Fields",
      "type": "array",
      "items": {
        "type": "string",
        "foo": "buzz"
      }
    }
  },
  "required": [
    "a_field",
    "some_fields"
  ]
}

With current master this outputs:

{
  "title": "Foo",
  "type": "object",
  "properties": {
    "a_field": {
      "title": "A Field",
      "foo": "buzz",
      "type": "string"
    },
    "some_fields": {
      "title": "Some Fields",
      "foo": "buzz",
      "type": "array",
      "items": {
        "type": "string",
        "foo": "buzz"
      }
    }
  },
  "required": [
    "a_field",
    "some_fields"
  ]
}

Diff:

 {
   "title": "Foo",
   "type": "object",
   "properties": {
     "a_field": {
       "title": "A Field",
-      "type": "string",
-      "foo": "buzz"
+      "foo": "buzz",
+      "type": "string"
     },
     "some_fields": {
       "title": "Some Fields",
+      "foo": "buzz",
       "type": "array",
       "items": {
         "type": "string",
         "foo": "buzz"
       }
     }
   },
   "required": [
     "a_field",
     "some_fields"
   ]
 }

Note the extra "foo" in some_fields - I think this is a regression?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug V1 Bug related to Pydantic V1.X
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant