Overriding default value makes __fields__ inconsistent with __annotations__

[kszucs]

Bug

Please complete:

• OS: OSX
• Python version import sys; print(sys.version): 3.7.3 | packaged by conda-forge | (default, Jul 1 2019, 14:38:56)
• Pydantic version import pydantic; print(pydantic.VERSION): 0.32.1

Please read the docs and search through issues to
confirm your bug hasn't already been reported before.

Where possible please include a self contained code snippet describing your bug:

from pydantic import BaseModel


class A(BaseModel):
    value: int = 1
        
        
class B(A):
    value = 'G'


print(A.__annotations__, A.__fields__)
# ({'value': int}, {'value': <Field(value type=int default=1)>})

print(A.__annotations__, A.__fields__)
# ({'value': int}, {'value': <Field(value type=str default='G')>})

The film type should be int in both cases. __annotations__ is empty in the metaclass so a type will be inferred for the field even without a type hint.

A solution would be to take the parents' annotations into consideration as well, or to introduce another config option to prevent inferring the type of the default value as the type hint for the field. I'm not sure how would the former one affect the rest of the features.

[dmontagu]

I think I’ve noticed this before with slightly different resulting issues but thought it might just be a limitation of python. Thanks for breaking this down.

I think there doesn’t need to be a config value, parent annotations should probably just be used if they exist; if you want to override you should probably just have to do it explicitly. I’m not sure if this would be hard to implement though.

[dmontagu]

This turned out to be a surprisingly easy fix. @kszucs I agree with your interpretation of what the intended behavior should be, but I defer to @samuelcolvin .

[kszucs]

@dmontagu just submitted a PR with a different approach, yours seem more robust

[dmontagu]

@kszucs yours also seems good to me, and has the benefit of only changing two lines 😄 and I thought mine was short.... I’m not sure which approach is more robust; I could see either way. We can wait for @samuelcolvin’s opinion. (I would be happy to close my PR in favor of @kszucs’s if his approach is preferable.)

[samuelcolvin]

[removed as I misread the issue].

[samuelcolvin]

I don't agree this is a bug, it's what I would expect to happen. However I'd consider raising an exception to make sure it's explicit.

Cancel that, I've just read through your question again and I see what you mean.

I think we should raise an eror if the subtype doesn't have an annotatoin.

[kszucs]

@samuelcolvin I think overriding the default value without altering the type hint would be useful, it enables a quite flexible declarative API:

from pydantic import BaseModel
from typing import List, Dict
from pathlib import Path


class Build(BaseModel):
    name: str
    builddir: Path
    env: Dict[str, str] = {}
    tags: List[str] = []
    

class LinuxBuild(Build):
    builddir = Path('/tmp/builddir')
    tags = ['linux']
    env = {
        'LD_LIBRARY_PATH': '/usr/local/lib'
    }
    

class OSXBuild(Build):
    builddir = Path('/var/temp/build')
    tags = ['osx']
    
    
lb = LinuxBuild(name='linux-test')
ob = OSXBuild(
    name='osx-test', 
    tags=['use', 'darwin', 'instead'], 
    env={'MACOSX_DEPLOYMENT_TARGET': '10.14'}
)

lb == <LinuxBuild name='linux-test' builddir=PosixPath('/tmp/builddir') env={'LD_LIBRARY_PATH': '/usr/local/lib'} tags=['linux']>
ob == <OSXBuild name='osx-test' builddir=PosixPath('/var/temp/build') env={'MACOSX_DEPLOYMENT_TARGET': '10.14'} tags=['use', 'darwin', 'instead']>

It works with #758 but fails with the current master.

[samuelcolvin]

I understand and use this myself.

The problem comes when the type on the child-model is different from the type on the parent model, eg. int vs .'G' in your above case - it's in this case where I think we should raise an error. Does that make sense?

[kszucs]

@samuelcolvin Dataclass permits to override the annotation in child classes:

from dataclasses import dataclass

@dataclass
class Uni:
    name: str = 'A'
    position: int = 1
        

@dataclass
class Byt(Uni):
    name: bytes = b'A'
    position: int = 1

Uni.__annotations__ == {'name': str, 'position': int}
Byt.__annotations__ == {'name': bytes, 'position': int}

Are you referring at, that the new default value might have different type than the annotation in the parent class?
Because a current behaviour of pydantic allows it:

from pydantic import BaseModel


class A(BaseModel):
    value: int = 1
        
        
class B(A):
    value: int = 'G'


B()

<B value='G'>

[kszucs]

Another example with more specific types down in the hierarchy:

class Car:
    pass


class SportCar(Car):
    pass


class CarStore(BaseModel):
    cars: List[Car]
    affordable: bool = True
        
        
class SportCarStore(CarStore):
    cars: List[SportCar]
    affordable: bool = False

[samuelcolvin]

I'm not opposed to overwriting the type or annotation in child classes, I'm just saying:

• M.__annotations__ and M.__fields__ should be consistent
• you shouldn't be able to override the type of a field implicitly on a child class

So

from pydantic import BaseModel

class A(BaseModel):
    value: int = 1

class B(A):
    value: str = 'G'

Would be fine,

But

from pydantic import BaseModel

class A(BaseModel):
    value: int = 1

class B(A):
    value = 'G'

Would raise an exception, something like "The type of B.value is implicitly changed by the default value, if you wish to change the type of this field, please use a type annotation: value: str = ...".

[kszucs]

@samuelcolvin sure, I agree with that :) - actually it would be great to have a validation for that.

[dmontagu]

I updated #758 to reflect this implementation, but there are some minor tradeoffs; I'll comment on them there.