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

narrowing only works on literals #8065

Closed
DetachHead opened this issue Jun 4, 2024 · 11 comments
Closed

narrowing only works on literals #8065

DetachHead opened this issue Jun 4, 2024 · 11 comments
Labels
as designed Not a bug, working as intended bug Something isn't working

Comments

@DetachHead
Copy link

def f(a: int | None, b: int):
    if a == 1:
        reveal_type(a)  # Literal[1]
    if a == b:
        reveal_type(a)  # int | None

playground

the type guard documentation mentions this:

  • x == ... and x != ...

i'm assuming ... refers to any expression. in that case, a == b should work. if that's not intended, then the documentation should be updated to clarify this

@DetachHead DetachHead added the bug Something isn't working label Jun 4, 2024
@erictraut
Copy link
Collaborator

Please stop filing low-quality bugs. You're wasting my time.

@erictraut erictraut closed this as not planned Won't fix, can't repro, duplicate, stale Jun 4, 2024
@erictraut erictraut added the as designed Not a bug, working as intended label Jun 4, 2024
@DetachHead
Copy link
Author

i hope you know that your attitude is the reason why so many people are switching to basedpyright

@xaviergmail
Copy link

xaviergmail commented Jun 11, 2024

"as designed"!? You can't be serious, right? What's low-quality about this bug report? It has everything you need and clearly highlights an issue with pyright.

@erictraut
Copy link
Collaborator

@xaviergmail, this is not a type guard pattern that is supported by pyright (and DetachHead knows this). So yes, pyright is working as designed here. For a complete list of type guard patterns supported by pyright, refer to this documentation.

We do occasionally add support for new type guard patterns, but each one requires custom logic, and they often negatively affect type analysis performance because they create false dependencies between otherwise-independent expressions. We therefore add support for new type guard patterns only when there is strong signal from pyright users. It needs to be a sufficiently common pattern to justify adding such support, and any performance impact must be mitigated or taken into consideration.

If you are interested in requesting a new feature in pyright, please open a new feature request.

@DetachHead
Copy link
Author

this is not a type guard pattern that is supported by pyright (and DetachHead knows this). So yes, pyright is working as designed here. For a complete list of type guard patterns supported by pyright, refer to this documentation.

what makes you assume i already knew this? i stated in my original post that my interpretation of the docs was that ... refers to some omitted value, which is usually what the ellipsis is used to represent. i see you have since updated the documentation like i suggested to clarify that in this case it refers to a literal ellipsis. this begs the question of why exactly you snapped at me for raising this issue if you admit it was a perfectly valid question worth updating the docs to clarify. (that said, i disagree that supporting only a small amount of hardcoded expressions to narrow types is the best solution, but that's beside the point)

i want you to know that i'm not trying to waste your time and all i've been trying to do is help improve pyright, because imo it's the only python type checker that's even worth the effort.

@erictraut
Copy link
Collaborator

@DetachHead, if you're interested in helping to improve pyright, I think there are ways you could do so more effectively. Your work on basedpyright is not well aligned with my work on pyright or the work of Microsoft's Pylance team. Nor is it well aligned with the efforts of others who are working to improve the Python type system and unify behaviors between static type checkers. I don't know how to contact you directly, but if you want to start a conversation, you're welcome to DM me on the Python discourse site or send me an email.

@DetachHead
Copy link
Author

Your work on basedpyright is not well aligned with my work on pyright

no offence but i feel like we are both very opinionated when it comes to deciding what features pyright should have. which is fine for a personal project but not for a type checker that would ideally be the standard used by everyone. for this reason i try to be open minded and support whichever features the community wants (as well as some additional non-standard features, but that's just a bonus, and users can disable them if they want)

there are quite a few issues i and many others have that i feel prevent pyright from being as widely used as it should be (mainly how difficult it is to install for average python developers because it's an npm package instead of a pypi one). i strongly believe that fixing such issues aligns with what most pyright users would want.

Nor is it well aligned with the efforts of others who are working to improve the Python type system and unify behaviors between static type checkers.

honestly i believe having multiple different type checkers is a massive waste of time and effort. the community's efforts would be much better spent improving the current best one (which is pyright in my opinion) and ditching the others.

the only argument for having multiple competing type checkers i can think of is competition. and what's the point of competition if they all have to strictly follow the spec?

or the work of Microsoft's Pylance team.

tbh i don't care if my work aligns with Microsoft's goals or not. in fact i'd rather it doesn't. their one and only goal seems to be to bring users to vscode to the complete detriment of the user experience for anybody who wants to use their type checker/language server anywhere else. one of my main goals is to improve the developer experience no matter what IDE you use, which fundamentally goes against what microsoft wants.

i would argue that pylance's work is less aligned to the typing community than mine.

I don't know how to contact you directly, but if you want to start a conversation, you're welcome to DM me on the Python discourse site or send me an email.

i'd like to keep this discussion public for now as i think it's important for transparency. however there is a public discord run by @KotlinIsland and myself that you are welcome to join if you prefer to continue the conversation there.

@InSyncWithFoo
Copy link
Contributor

InSyncWithFoo commented Jun 25, 2024

(No comments on the meta-discussion above.)

Considering that one can inherit from int, such a type guard would be unsafe:

from typing import Self, Any, reveal_type

class NoneInt(int):
	def __new__(cls, v: Any) -> Self:
		return super().__new__(cls, v)
	def __eq__(self, other: object) -> bool:
		return other is None or super().__eq__(other)
def f(a: int | None, b: int) -> None:
	if a == b:
		reveal_type(a)  # int at type checking time, yet None at runtime

f(None, NoneInt(42))

@DetachHead
Copy link
Author

true. i think that should also be mentioned in the documentation to explain why narrowing this way doesn't work (especially since it does work in mypy)

if that's the reason this functionality isn't supported, i think it should at least work on classes decorated with @final that don't have an __eq__:

from typing import final

@final
class Int(int):
    pass

def f(a: Int | None, b: Int):
    if a == 1:
        reveal_type(a)  # Literal[1]
    if a == b:
        reveal_type(a)  # should be `Int`

as far as i can tell this is completely safe because it's impossible for this Int type to have an __eq__

@InSyncWithFoo
Copy link
Contributor

Currently int.__eq__() is defined as:

class int:
    ...
    def __eq__(self, value: object, /) -> bool: ...

Comparisons with ints are internally special-cased, but this doesn't apply to their subclasses (playground):

from typing import final

@final
class Int(int):
    pass
a = int(0)
b = Int(0)

if a == '3.0':      # error: "int" and "Literal['3.0']" have no overlap
    reveal_type(a)

if b == '3.0':      # fine
    reveal_type(b)  # Int

If this were to be implemented, the special-casing would need to be inheritable. I wonder how much of a breaking change, both code-wise and user-wise, that would be.

@bukzor
Copy link

bukzor commented Jul 21, 2024

The docs were updated as requested in
8e1797c

x == ... was intended as the literal code -- equality of x and the ellipsis token ....

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 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants