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

PyRight doesn't narrow type based on hasattr #6717

Closed
robertmaxton42 opened this issue Dec 12, 2023 · 8 comments
Closed

PyRight doesn't narrow type based on hasattr #6717

robertmaxton42 opened this issue Dec 12, 2023 · 8 comments
Labels
enhancement request New feature or request

Comments

@robertmaxton42
Copy link

robertmaxton42 commented Dec 12, 2023

Describe the bug

def foo(h: list | float):
    if hasattr(h, '__getitem__') and h[0] == 0:
        pass

PyRight puts a red squiggle under h in h[0], saying ""__getitem__" method not defined on type "float". While canonically determining whether an object is indexable in Python is difficult enough that the Python documentation itself says that the only way to be sure is to try it and find out, in this case PyRight requires one specific method to exist, so it should also be able to tell that I've guaranteed that the method exists earlier in the line. It would be extra-nice if it could ascribe h to a duck type whose only known feature is that it has __getitem__, but at the least in this context it should be able to infer that, between the options list and float, h must be a list. (Moving h[0] == 0 into a new if doesn't help.)

VS Code extension or command-line
PyLance v2023.11.10 in VSCode on WSL

@robertmaxton42 robertmaxton42 added the bug Something isn't working label Dec 12, 2023
@erictraut erictraut added enhancement request New feature or request and removed bug Something isn't working labels Dec 12, 2023
@erictraut
Copy link
Collaborator

This isn't a supported type guard pattern, so no type narrowing is performed in this case. For a full list of supported type guard patterns, refer to this documentation.

Each new type guard pattern requires specialized logic and potentially slows down overall type analysis. We do occasionally add support for new type guard patterns but only if we have strong signal that it's a common pattern. I don't think that hasattr qualifies, but we could consider adding this in the future if there's sufficient feedback from users.

In the meantime, the recommended approach is to use a "user-defined type guard" using the TypeGuard mechanism introduced in PEP 647.

I'm going to close this feature request for now because we have no current plans to add support for this type guard pattern.

@erictraut erictraut closed this as not planned Won't fix, can't repro, duplicate, stale Dec 12, 2023
@robertmaxton42
Copy link
Author

I mean, hasattr is the thing the Python docs say you should do in performance-sensitive code in the typing docs themselves: see https://docs.python.org/3/library/typing.html#typing.runtime_checkable, the last gray box. So, to some extent it's "the correct thing to be doing"?

But sure, I guess I can use a TypeGuard for now.

@erictraut
Copy link
Collaborator

For comparison, mypy (another popular type checker) also does not support hasattr as a type guard pattern. I don't think this is a common idiom in Python.

The common approach is to use isinstance.

def foo(h: list | float):
    if isinstance(h, list) and h[0] == 0:
        pass

The documentation you quoted applies to runtime protocols. The types you used in your example (list and float) are not protocol classes.

@robertmaxton42
Copy link
Author

Well, sure, but my actual use case would have liked to use isinstance(h, Iterable) there, for example, and resorted to hasattr instead. (I suppose I should've used hasattr(h, "__iter__") too, oops.)

@MetRonnie
Copy link

I get that there may be reasons not to implement this, but this example is rather irritating

def _pytest_passed(request: pytest.FixtureRequest) -> bool:
    """Returns True if the test(s) a fixture was used in passed."""
    if hasattr(request.node, '_function_outcome'):
        return request.node._function_outcome.outcome in {'passed', 'skipped'}
#                           ~~~~~~~~~~~~~~~~~
# Cannot access member "_function_outcome" for type "Item"
#  Member "_function_outcome" is unknownPylancereportAttributeAccessIssue

@JonathanPlasse
Copy link

MyPy supports it at least partially.

@Avasam
Copy link

Avasam commented Apr 16, 2024

Similarly I was surprised that hasattr doesn't at least narrow None out of a union in this example: https://github.com/pypa/setuptools/pull/4262/files#diff-e3d3d454fa3a072c9f46f8affa27513892fbc2d245e87f57a5927a7be851de05R1639

Having an attribute doesn't necessarily mean you got the expected type, but at least you know it can't be None (which has no attributes).
Slightly different special-casing than #5964 (comment) as it isn't generalised. But at least I'm signaling that I'd like this as a user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement request New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants