pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.31k stars 1.13k forks source link

false positive `no-member` warnings in pydantic 2.0 #8894

Open tylerflex opened 1 year ago

tylerflex commented 1 year ago

Bug description

Description

I'm using pydantic.v1 to transition our codebase to pydantic 2.* but doing so has resulted in many pylint warnings due to the use of pydantic.v1.Field() in defining the fields of my class.

x : int = pydantic.v1.Field(...)`

it seems that pylint interprets self.x as a ModelField instead of an int and therefore creates many warnings throughout the entire codebase.

Note that this doesn't occur if I leave out the pd.Field() definition.

x : int

Any thoughts on how to get around this issue would be really appreciated! Code example below showing the issue.

(I am running pylint-2.17.5 and Python 3.10.9)

Thanks

Code Snippet

""" module docstring """
import pydantic.v1 as pydantic

class ClassA(pydantic.BaseModel):
    """Class A"""

    field_a : float

class ClassB(pydantic.BaseModel):
    """Class B"""

    # doesnt trigger pylint
    obj_a: ClassA

    # triggers Instance of 'FieldInfo' has no 'field_a' member (no-member) from below method
    obj_a: ClassA = pydantic.Field(...)

    def get_field_a(self):
        """Grab the 'field_a' from 'self.obj_a'."""
        return self.obj_a.field_a

# simple test to show that the method works as expected
FIELD_A = 1.0
obj_a = ClassA(field_a=FIELD_A)
obj_b = ClassB(obj_a=obj_a)
assert obj_b.get_field_a() == FIELD_A

Python, Pydantic & OS Version

pydantic version: 2.1.1
        pydantic-core version: 2.4.0
          pydantic-core build: profile=release pgo=false mimalloc=true
                 install path: /Users/twhughes/.pyenv/versions/3.10.9/lib/python3.10/site-packages/pydantic
               python version: 3.10.9 (main, Dec 30 2022, 09:25:19) [Clang 14.0.0 (clang-1400.0.29.202)]
                     platform: macOS-12.5.1-arm64-arm-64bit
     optional deps. installed: ['typing-extensions']

Configuration

No response

Command used

pylint a.py

Pylint output

************* Module lint_test
lint_test.py:22:15: E1101: Instance of 'FieldInfo' has no 'field_a' member (no-member)

Expected behavior

***** Module lint_test

Pylint version

pylint 2.17.5
astroid 2.15.6
Python 3.10.9 (main, Dec 30 2022, 09:25:19) [Clang 14.0.0 (clang-1400.0.29.202)]

OS / Environment

MacOS

Additional dependencies

pip install pydantic==2.1.0

tylerflex commented 1 year ago

See the issue in pydantic here

mbyrnepr2 commented 1 year ago

Hi @tylerflex. From the code in the linked issue I see a problem. Here is the snipped I am referring to:

class ClassB(pydantic.BaseModel):
    """Class B"""

    # doesnt trigger pylint
    obj_a: ClassA

    # triggers Instance of 'FieldInfo' has no 'field_a' member (no-member) from below method
    obj_a: ClassA = pydantic.Field(...)

Neither of these lines are triggering the Pylint message; that is happening on this line:

return self.obj_a.field_a

In the case of obj_a: ClassA, Pylint doesn't try to infer from a type annotation like this and so nothing is emitted. Any value can be given instead of field_a in self.obj_a.field_a and the message would not be emitted either. In the case of obj_a: ClassA = pydantic.Field(...), Pylint emits the message because pydantic.Field() instance doesn't have a field_a member (At least I don't think it does :D ?).

In summary I would say this is not a false positive since the expected behaviour of using the type-annotation as a source of truth for where the member lives is not part of Pylint currently. There could be a specific issue for this in the tracker somewhere and I think this feature may be discussed at some point but probably deserves its own issue anyway. Would be curious to know others' opinions on adding that feature.

tylerflex commented 1 year ago

Hi @mbyrnepr2 thanks for taking a look at this.

Neither of these lines are triggering the Pylint message; that is happening on this line: return self.obj_a.field_a

Yea this is my understanding as well. And yea pydantic.Field() is (I suppose) a pydantic.ModelField instance, so it basically thinks that self.obj_a is a pydantic.ModelField instance rather than a ClassA instance. Essentially it seems to be inferring type from the assigned value rather than the type annotation?

The issue is that this snippet works with pydantic 1.* without pylint warnings, so my guess is that something in pylint was previously able to infer the type of self.obj_a properly before pydantic got upgraded to 2.0.

According to the pydantic people from the other issue, there is no warning with VSCode PyLint/Pyright or mypy so maybe there's just some edge case in pylint that needs to be handled to take care of this?

Again, really appreciate you looking into this!

mbyrnepr2 commented 1 year ago

inferring type from the assigned value rather than the type annotation

👍

If I use the earlier Pydantic version there are import errors:

x.py:2:0: E0401: Unable to import 'pydantic.v1' (import-error)
x.py:2:0: E0611: No name 'v1' in module 'pydantic' (no-name-in-module)
pip freeze |grep pydantic
pydantic==1.10.12
pydantic_core==2.4.0

Edit: Ok if it's import pydantic then the run is clean. Let me see...

tylerflex commented 1 year ago

yea for the earlier pydantic version, remove the v1 and It should work. (pydantic.v1 is just a way to import pydantic 1. features from 2.)

mbyrnepr2 commented 1 year ago

Pylint wasn't able to infer the pydantic.Field object in the earlier Pydantic version but it is able to infer it using the latest Pydantic version (I'm using the same latest Pylint version in both scenarios). In the latter case, Pylint gives the correct message since that field_a attribute doesn't exist; the reason Pylint didn't emit the message with the earlier version of Pydantic is because it wasn't able to infer and "gave up".

This sort of thing happens quite a bit with Pylint and 3rd-party packages since sometimes the 3rd-party code will change and suddenly Pylint doesn't understand it and can't infer anymore; the opposite sometimes happens too :D. This is why there is an ecosystem of Pylint plugins out there, to help Pylint understand 3rd-party packages better and reduce the occurrence of false positives etc. There is one out there for pydantic too (pylint-pydantic). In your case there is a false negative with the earlier Pydantic version.

Lithimlin commented 1 year ago

I'm actually still having this issue when using Microsoft's pylint extension (v2023.6.0) in VS Code (OSS version 1.81.1), except this time pylint thinks that the fields are instances of pydantic.FieldInfo

Pierre-Sassoulas commented 1 year ago

What is your version of pydantic ?

Lithimlin commented 1 year ago

pydantic 2.3.0 core version 2.6.3

mbyrnepr2 commented 1 year ago

There’s probably confusion here. I think it’s not specifically an issue with Pylint understanding Pydantic. I think the bigger issue is Pylint not understanding the type from the annotation. That is why this was invalid (the part about pydantic), this issue has been mentioned numerous times on different issues and I’m not against that at all, it just needs it’s own issue. There’s even an open PR WIP that tries to improve this https://github.com/pylint-dev/pylint/pull/8778

Pierre-Sassoulas commented 1 year ago

Feel free to close again Marc, I just wanted to make sure the discussion was not lost. (Type annotation taken into account issue : #4813 )

mbyrnepr2 commented 1 year ago

I’m also referring to the discussion in the Pydantic issue linked above. I think the wrong impression was given. It’s fine to leave open, better safe than sorry.

Lithimlin commented 1 year ago

If it helps, here's a minimal example:

from pydantic import BaseModel, Field

class Foo(BaseModel):
    bar: str
    baz: int = Field(default=0)
    bat: dict[str, int] = Field(default={}, required=False)

def main():
    foo = Foo(bar="foo", baz=1, bat={"a": 1, "b": 2})

    print(foo.bar, foo.baz)
    for k, v in foo.bat.items():
        print(k, v)

if __name__ == "__main__":
    main()

I then get the following error:

Instance of 'FieldInfo' has no 'items' member; Pylint(E1101:no-member) [Ln 14, Col 17]

I have adjusted my pylint settings as follows as I saw somewhere that that could help:

{
    "pylint.args": [
        "--load-plugins",
        "pylint_pydantic",
        "--generated-members",
        "from-json,query"
    ]
}
WYishai commented 1 year ago

I can add that I see the same error when I run Pylint using Python 3.11, even when the version of Pydantic is 1.9.2 (and not only in >=2.0.0).

It's working as expected for Python 3.9.11 and Pydantic 1.9.2 (no error from Pylint). It's showing this error for Python 3.9.11 (only with Pydantic 2.3.0) It's showing this error for Python 3.11.4 (with Pydantic 1.9.2 or Pydantic 2.3.0)

Tested with Pylint versions (and found the same behavior): 2.17.5, 2.16.4, 2.15.10, 2.14.5, 2.13.9, 2.12.2

The minimal code I used to reproduce the issue:

from pydantic import BaseModel, Field

class MyModel(BaseModel):
    field_a: str = Field(default='aaa')

_ = MyModel().field_a.replace('a', 'b')

Error message: the/path/to/my/file.py:6:4: E1101: Instance of 'FieldInfo' has no 'replace' member (no-member)

mbyrnepr2 commented 1 year ago

The examples provided in the previous two comments won't emit no-member if the pylint-pydantic plugin is used: pylint --load-plugins pylint_pydantic example.py. The original v1 example still emits the message, however; perhaps the pylint-pydantic maintainers would be open to addressing that case also?

Timmmm commented 10 months ago

You don't need Field to reproduce this:

from pydantic import BaseModel

class Config(BaseModel):
    options: "list[str]"

config = Config(options=[])
config.options += ["hello"]