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

Type inference on owner argument to __set_name__ #7039

Closed
sg495 opened this issue Jan 19, 2024 · 4 comments
Closed

Type inference on owner argument to __set_name__ #7039

sg495 opened this issue Jan 19, 2024 · 4 comments
Labels
as designed Not a bug, working as intended

Comments

@sg495
Copy link

sg495 commented Jan 19, 2024

Because of the circumstances under which __set_name__ is invoked, it should be possible to perform type inference on its owner argument, in a way which could be used to statically typecheck other components of a descriptor.

As a brief motivating example, below is a sketch of descriptor for a validated mutable attribute of integer type:

from __future__ import annotations
from typing import Any, Generic, Protocol, Self, Type, TypeVar, cast, overload

InstanceT = TypeVar("InstanceT")
InstanceT_contra = TypeVar("InstanceT_contra", contravariant=True)

class Validator(Protocol[InstanceT_contra]):

    def __call__(self, instance: InstanceT_contra, value: int, /) -> bool:
        ...

class IntAttr(Generic[InstanceT]):

    __validator: Validator[InstanceT]
    __name: str

    def __init__(self, validator: Validator[InstanceT]) -> None:
        self.__validator = validator

    def __set_name__(self, owner: Type[InstanceT], name: str) -> None:
        self.__name = name

    @overload
    def __get__(self, instance: None, _: Type[Any]) -> Self:
        ...

    @overload
    def __get__(self, instance: InstanceT, _: Type[Any]) -> int:
        ...

    def __get__(self, instance: InstanceT|None, _: Type[Any]) -> int | Self:
        if instance is None:
            return self
        return cast(int, getattr(instance, f"__{self.__name}"))

    def __set__(self, instance: InstanceT, value: int) -> None:
        if not self.__validator(instance, value):
            raise ValueError()
        setattr(instance, f"__{self.__name}", value)

class C:

    x = IntAttr(lambda self, value: self.validate(value)) # Pylance error

    def validate(self, value: int) -> bool:
        return value >= 10

In Pylance with strict typechecking rules, the definition of the x descriptor raises the following errors:

reportUnknownMemberType
  Type of "validate" is unknown 
reportUnknownLambdaType
  Return type of lambda is unknown
reportGeneralTypeIssues
  Cannot access member "validate" for type "object*"
    Member "validate" is unknown

Explicitly providing a hint for the InstanceT type removes the errors:

class C:

    x = IntAttr["C"](lambda self, value: self.validate(value)) # Errors

    def validate(self, value: int) -> bool:
        return value >= 10

Performing inference on the owner argument to IntAttr.__set_name__ in the context of class C would ideally result in C being inferred as a value for InstanceT, allowing for static typecheking of the validator lambda function without the need for an explicit type hint.

Related issues for Mypy:

Sister issue for Mypy: python/mypy#16796

@debonte debonte transferred this issue from microsoft/pylance-release Jan 19, 2024
@erictraut
Copy link
Collaborator

Pyright is working as designed here. A type checker cannot make the assumptions that you seem to think it should in this case. As you point out, you can remove the ambiguity by providing an explicit specialization for IntAttr.

@erictraut erictraut added the as designed Not a bug, working as intended label Jan 19, 2024
@sg495
Copy link
Author

sg495 commented Jan 19, 2024

In case it wasn't clear: this was a feature request, not a bug report. Could you please explain your statement:

A type checker cannot make the assumptions that you seem to think it should in this case.

For descriptors defined in a class, the class context is known statically, and that's the same class that is passed as owner to __set_name__. It would be useful to understand the failure modes you have in mind, and how common they are compared to the situation I'm describing.

PS: Having to provide explicit specialisation is brittle, because it has to be done via a forward reference. It is also unsightly and unnecessarily laborious, but perhaps that's a matter of taste.

@erictraut
Copy link
Collaborator

The specialized type of an object is established at the time it is constructed. The __set_name__ hook has nothing to do with the construction of the object; it is called later.

The error that you're seeing is because there's insufficient information for a type checker to infer the type of the lambda that you're passing to the constructor.

@sg495
Copy link
Author

sg495 commented Jan 19, 2024

I'm aware of all of that, thank you (although I might disagree with your first sentence, depending on the specific meaning of "time it is constructed").

Here, the class context in which the descriptor object is constructed is known statically, as is the way in which __set_name__ will be invoked: the information is all available.

The fact that a specific type-checker might not have access to that information is a design choice/limitation of the type-checker, hence the feature request...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as designed Not a bug, working as intended
Projects
None yet
Development

No branches or pull requests

3 participants