From c616dcb4530c2feb980a0bc3470b18b391370f52 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Wed, 11 May 2022 19:00:37 +0100 Subject: [PATCH] test pyright with pydantic (#3972) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test pyright with pydantic * rename file to avoid pytest running it * try another name 😴 * add docs about BaseSettings and Field * add change --- .github/workflows/ci.yml | 9 ++++++ Makefile | 4 +++ changes/3972-samuelcolvin.md | 1 + docs/visual_studio_code.md | 48 ++++++++++++++++++++++++++++++-- tests/pyright/pyproject.toml | 4 +++ tests/pyright/pyright_example.py | 38 +++++++++++++++++++++++++ 6 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 changes/3972-samuelcolvin.md create mode 100644 tests/pyright/pyproject.toml create mode 100644 tests/pyright/pyright_example.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a3db405bec..0b96f7d13e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,6 +37,15 @@ jobs: - name: check dist run: make check-dist + - name: install node for pyright + uses: actions/setup-node@v3 + with: + node-version: '14' + + - run: npm install -g pyright + + - run: make pyright + docs-build: runs-on: ubuntu-latest steps: diff --git a/Makefile b/Makefile index 46060befb6..cf846064ea 100644 --- a/Makefile +++ b/Makefile @@ -54,6 +54,10 @@ check-dist: mypy: mypy pydantic +.PHONY: pyright +pyright: + cd tests/pyright && pyright + .PHONY: test test: pytest --cov=pydantic diff --git a/changes/3972-samuelcolvin.md b/changes/3972-samuelcolvin.md new file mode 100644 index 0000000000..42f2498277 --- /dev/null +++ b/changes/3972-samuelcolvin.md @@ -0,0 +1 @@ +Typing checking with pyright in CI, improve docs on vscode/pylance/pyright. diff --git a/docs/visual_studio_code.md b/docs/visual_studio_code.md index 98c58fb01f..6439a5d3df 100644 --- a/docs/visual_studio_code.md +++ b/docs/visual_studio_code.md @@ -130,10 +130,18 @@ Below are several techniques to achieve it. You can disable the errors for a specific line using a comment of: -``` +```py # type: ignore ``` +or (to be specific to pylance/pyright): + +```py +# pyright: ignore +``` + +([pyright](https://github.com/microsoft/pyright) is the language server used by Pylance.). + coming back to the example with `age='23'`, it would be: ```Python hl_lines="10" @@ -146,7 +154,7 @@ class Knight(BaseModel): color: str = 'blue' -lancelot = Knight(title='Sir Lancelot', age='23') # type: ignore +lancelot = Knight(title='Sir Lancelot', age='23') # pyright: ignore ``` that way Pylance and mypy will ignore errors in that line. @@ -243,10 +251,44 @@ The specific configuration **`frozen`** (in beta) has a special meaning. It prevents other code from changing a model instance once it's created, keeping it **"frozen"**. -When using the second version to declare `frozen=True` (with **keyword arguments** in the class definition), Pylance can use it to help you check in your code and **detect errors** when something is trying to set values in a model that is "frozen". +When using the second version to declare `frozen=True` (with **keyword arguments** in the class definition), +Pylance can use it to help you check in your code and **detect errors** when something is trying to set values +in a model that is "frozen". ![VS Code strict type errors with model](./img/vs_code_08.png) +## BaseSettings and ignoring Pylance/pyright errors + +Pylance/pyright does not work well with [`BaseSettings`](./usage/settings.md) - fields in settings classes can be +configured via environment variables and therefore "required" fields do not have to be explicitly set when +initialising a settings instance. However, pyright considers these fields as "required" and will therefore +show an error when they're not set. + +See [#3753](https://github.com/samuelcolvin/pydantic/issues/3753#issuecomment-1087417884) for an explanation of the +reasons behind this, and why we can't avoid the problem. + +There are two potential workarounds: + +* use an ignore comment (`# pylance: ignore`) when initialising `settings` +* or, use `settings.parse_obj({})` to avoid the warning + +## Adding a default with `Field` + +Pylance/pyright requires `default` to be a keyword argument to `Field` in order to infer that the field is optional. + +```py +from pydantic import BaseModel, Field + + +class Knight(BaseModel): + title: str = Field(default='Sir Lancelot') # this is okay + age: int = Field(23) # this works fine at runtime but will case an error for pyright + +lance = Knight() # error: Argument missing for parameter "age" +``` + +Like the issue with `BaseSettings`, this is a limitation of dataclass transforms and cannot be fixed in pydantic. + ## Technical Details !!! warning diff --git a/tests/pyright/pyproject.toml b/tests/pyright/pyproject.toml new file mode 100644 index 0000000000..991559aeaf --- /dev/null +++ b/tests/pyright/pyproject.toml @@ -0,0 +1,4 @@ +[tool.pyright] +extraPaths = ['../..'] +reportUnnecessaryTypeIgnoreComment = true +pythonVersion = '3.10' diff --git a/tests/pyright/pyright_example.py b/tests/pyright/pyright_example.py new file mode 100644 index 0000000000..0819afc3c4 --- /dev/null +++ b/tests/pyright/pyright_example.py @@ -0,0 +1,38 @@ +""" +This file is used to test pyright's ability to check pydantic code. + +In particular pydantic provides the `@__dataclass_transform__` for `BaseModel` +and all subclasses (including `BaseSettings`), see #2721. +""" + +from typing import List + +from pydantic import BaseModel, BaseSettings, Field + + +class MyModel(BaseModel): + x: str + y: List[int] + + +m1 = MyModel(x='hello', y=[1, 2, 3]) + +m2 = MyModel(x='hello') # pyright: ignore + + +class Knight(BaseModel): + title: str = Field(default='Sir Lancelot') # this is okay + age: int = Field(23) # this works fine at runtime but will case an error for pyright + + +k = Knight() # pyright: ignore + + +class Settings(BaseSettings): + x: str + y: int + + +s1 = Settings.parse_obj({}) + +s2 = Settings() # pyright: ignore[reportGeneralTypeIssues]