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

Pydantic Field alias - wrong invalid string reporting #9386

Closed
yann-combarnous opened this issue Nov 5, 2024 · 5 comments
Closed

Pydantic Field alias - wrong invalid string reporting #9386

yann-combarnous opened this issue Nov 5, 2024 · 5 comments
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working

Comments

@yann-combarnous
Copy link

Describe the bug
For Pydantic field alias, valid strings are reported as being invalid.

Code or Screenshots

class Resource(BaseModel):
    odataType: str = Field(alias="@odata.type")  # type: ignore[reportGeneralTypeIssues]

alias is reported as str | None, but pyright raises reportGeneralTypeIssues

pydantic = "2.9.2"

VS Code extension or command-line
Using VSCode Pylance, also reported using jakebailey/pyright-action, using pylance-version: latest.

This is a new issue, was not reported prior to last week.

@yann-combarnous yann-combarnous added the bug Something isn't working label Nov 5, 2024
@erictraut
Copy link
Collaborator

erictraut commented Nov 5, 2024

This change was intentional. See this issue.

How would you construct an instance of the Resource class in your example? You cannot pass a keyword argument named "@odata.type" because this isn't a valid identifier.

Part of the challenge here is that the alias field was added to the dataclass_transform facility in the typing spec as an extension to the stdlib dataclass, which doesn't support alias. That means we can't copy the semantics of dataclass here. The alias field parameter was originally added to the spec for the benefit of attrs, but it looks like pydantic supports it. Unfortunately, their semantics differ in small ways.

Right now, the typing spec says "alias is an optional str parameter that provides an alternative name for the field. This alternative name is used in the synthesized __init__ method." By my reading, this implies the alternative name must be a valid parameter name.

@Viicos, do you have any thoughts on this?

@Viicos
Copy link

Viicos commented Nov 5, 2024

In Pydantic, you can also construct a model using the model_validate_* classmethods:

class Resource(BaseModel):
    odataType: str = Field(alias="@odata.type")

resource = Resource.model_validate({'@odata.type': '...'})

Where it makes sense to use invalid Python identifiers as keys.

Here are a couple options that could be considered:

  • Do not error when the alias is not a valid identifier. After all, users can still do Resource(**{'@odata.type': '...'}). This is similar to unpacked typed dictionaries, where the following currently works with no pyright error (playground):
from typing import TypedDict, Unpack


TD = TypedDict('TD', {'@odata.type': str})


def func(**kwargs: Unpack[TD]): ...


func(**{'@odata.type': '...'})
  • Add a new kind of error so that these can easily be ignored at the project level. However, I don't know what's the policy when it comes to adding new/changin error codes in pyright.

@yann-combarnous: alternatively, you can set the Field as Annotated metadata. Pyright won't error, but won't be able to pick up other Field arguments, such as default values, etc:

from typing import Annotated

class Resource(BaseModel):
    odataType: Annotated[str, Field(alias="@odata.type")] = "a"
    # Note that pyright won't be able to understand that the following field
    # has a default value:
    test: Annotated[str, Field(default="a")]

Resource()  # pyright error, missing value for argument `test`

@erictraut
Copy link
Collaborator

erictraut commented Nov 5, 2024

Oh, that's a good point about using an unpacked dictionary (as in Resource(**{'@odata.type': '...'})).

@debonte, based on this feedback, I think I will undo the change I made for #9220. This isn't an error condition unless/until you attempt to use the illegal identifier as as keyword argument. Pyright already flags that as an error. The error check I added for #9220 is therefore a false positive.

If I revert the change for #9220, the completion provider logic will then need to become smarter to avoid recommending an illegal keyword argument identifier.

Here's a similar case — involving a TypedDict rather than a dataclass_transform class — where the completion provider produces problematic completion suggestions:

TD1 = TypedDict("TD1", {"@illegal param": int})
td1 = TD1(**{"@illegal param": 3})

@debonte
Copy link
Collaborator

debonte commented Nov 5, 2024

Sounds good, thanks.

erictraut added a commit that referenced this issue Nov 5, 2024
…aclass_transform` field. This isn't an illegal condition, so the resulting error was a false positive. Instead, changed completion provider to not suggest the illegal identifier as a valid keyword argument. This addresses #9386, #9220, #9278.
erictraut added a commit that referenced this issue Nov 5, 2024
…aclass_transform` field. This isn't an illegal condition, so the resulting error was a false positive. Instead, changed completion provider to not suggest the illegal identifier as a valid keyword argument. This addresses #9386, #9220, #9278. (#9390)
@erictraut erictraut added the addressed in next version Issue is fixed and will appear in next published version label Nov 5, 2024
@erictraut
Copy link
Collaborator

This is addressed in pyright 1.1.388.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addressed in next version Issue is fixed and will appear in next published version bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants