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

FR: Type narrowing for x is C #7642

Open
jbwdevries opened this issue Oct 5, 2019 · 19 comments
Open

FR: Type narrowing for x is C #7642

jbwdevries opened this issue Oct 5, 2019 · 19 comments

Comments

@jbwdevries
Copy link

jbwdevries commented Oct 5, 2019

This is a feature request.

Example unit test:

[case testIsNarrowAny]
from typing import Any, Type, Union

def narrow_any_to_str_then_reassign_to_int() -> None:
    v = 1 # type: Union[Type[Any], int]

    if v is Any:
        reveal_type(v)  # N: Revealed type is 'Type[Any]'
        v = 2
        reveal_type(v)  # N: Revealed type is 'int'

[builtins fixtures/isinstance.pyi]

For a project I'm working on, it would be help readability if I could write code like the above. However, mypy does not perform type narrowing in this case, so that results in a lot of type warnings.

Note: This is different from type(x) is C. In this case, x is the type instance itself, so no subclasses are involved.

@jbwdevries
Copy link
Author

I'm guessing I'd have to modify checkexpr.py to add similar functionality like is already found in visit_call_expr_inner to visit_comparison_expr, but not sure how to proceed from there. I would appreciate any tips or suggestions on whether this can be implemented, and how.

@gvanrossum
Copy link
Member

Did you mean to request this specifically for Any? The issue's subject mentions x is C which seems to be quite different than x is Any which you have in your example code.

@jbwdevries
Copy link
Author

Using Any was my attempt to make the unit test as small as possible.

A more representative use case would be as follows:

from typing import Type, Union

class Missing:
    pass

def my_func(a: Union[Type[Missing], int]) -> None:
    if a is Missing:
        reveal_type(a)  # N: Revealed type is 'Type[foo2.Missing]'
    else:
        reveal_type(a)  # N: Revealed type is 'builtins.int'

In the current version of mypy the two reveal_types under is Missing are revealed as Union[Type[foo2.Missing], builtins.int] instead.

I could make it work by changing my project around to use it like this:

from typing import Type, Union

class Missing:
    pass

def my_func(a: Union[Missing, int]) -> None:
    if isinstance(a, Missing):
        reveal_type(a)  # N: Revealed type is 'foo2.Missing'
    else:
        reveal_type(a)  # N: Revealed type is 'builtins.int'

However, I would prefer the readability of my first example, if at all possible.

@JelleZijlstra
Copy link
Member

In your first example, it would be incorrect in general for mypy to infer int in the else branch, because a may be a subclass of Missing, not Missing itself.

@jbwdevries
Copy link
Author

Hm, this is because once I use a class to define a type, I introduce the possibility of inheritance, yes? But I have no use for inheritance. Can you tell me how to change the example so there is no risk of inheritance?

@jbwdevries
Copy link
Author

I can't do it like this either:

from typing import Type, Union

class MissingType:
    pass

Missing = MissingType()

def my_func(a: Union[MissingType, int]) -> None:
    if a is Missing:
        reveal_type(a)  # N: Revealed type is 'Type[foo2.MissingType]'
    else:
        reveal_type(a)  # N: Revealed type is 'builtins.int'

Since mypy has no way of knowing that Missing is (or at least should be) the only possible instance of MissingType.

I guess I'm trying to do something like Haskell's Maybe type:

data Maybe a = Just a | Nothing

In this case, Nothing is something on its own, it's not a class that can be instantiated or subclassed.

@JelleZijlstra
Copy link
Member

You can do this if you just use None for Nothing: mypy does know to special-case is None.

@jbwdevries
Copy link
Author

I'm aware, however, None is a valid value separate from Missing or Invalid or any other such types I may want to introduce.

@gvanrossum
Copy link
Member

So tell me if I'm wrong. You're trying to introduce a singleton value other than None, and you want the type checker to understand code that excludes the singleton value.

I think this is a reasonable request. There are two aspects to the puzzle: how to make testing for the singleton value look intuitive, and how to make the type checker understand it. When we have solved both we can recommend a way to define singletion values and how to test for them.

Traditionally this would be done like this:

nothing = object()
def foo(arg=nothing):
    if arg is nothing:
        <default case>
    else:
        <regular case>

How can we do it type-safely? Apparently there isn't an easy way to spell this without isinstance(), which I agree is too verbose.

Maybe we can may mypy realize that enums (if they define any values) cannot be subclassed and have unique values, and then the "nothing" marker could be a member of a one-element enum:

import enum
class Nothingness(enum.Enum):
    Nothing = 0
Nothing = Nothingness.Nothing
def foo(arg: Union[Nothingness, int]):
    if arg is Nothing:
        <default case>
    else:
        <regular case>

@ilevkivskyi
Copy link
Member

@gvanrossum The enum solution already works in mypy (except aliasing enum members will likely not work, unless you make variable a literal type or final). Taking into account this, should we still keep this open, or is it a satisfactory solution?

@Michael0x2a
Copy link
Collaborator

Michael0x2a commented Oct 7, 2019

Maybe it might be sufficient to just add a check for this inside https://github.com/python/mypy/blob/master/mypy/checker.py#L4720?

Specifically, we could perhaps modify that check so that it also returns true if we have some type of the form Type[T] where T is some class that was marked as final (via the typing.final/typing_extensions.final decorator).

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 7, 2019

What about also special casing is against a single-element enum value to be always true? This way the aliasing may work without using final or literal types.

Making Type[X] a singleton if X is final sounds like a good idea.

Whatever we decide to implement, it would be good to document this. The supported way to do this is probably not obvious to all users, since it looks like the object() idiom can't be supported.

@gvanrossum
Copy link
Member

gvanrossum commented Oct 7, 2019 via email

@ilevkivskyi
Copy link
Member

What about also special casing is against a single-element enum value to be always true? This way the aliasing may work without using final or literal types.

Making Type[X] a singleton if X is final sounds like a good idea.

Both ideas for special-casing makes sense. Now we need to find a volunteer to implement them :-)

@jbwdevries
Copy link
Author

Hm, the example from @gvanrossum with enums DOES work if the Nothing is made Final, which turns it into a Literal[enum] which has explicit support in is_singleton_type already:

from typing import Type, Union
from typing_extensions import Final

import enum

class Nothingness(enum.Enum):
    Nothing = 0

Nothing: Final = Nothingness.Nothing

def foo(arg: Union[Nothingness, int]):
    if arg is Nothing:
        reveal_type(arg)
    else:
        reveal_type(arg)

Adding a second type to the enum values in the one value being discarded, leaving only literals for the remaining enum values.

I'm not sure this is actually documented, though. It's tested in testEnumReachabilityPEP484Example1 (maybe others), that's how I found it.

Extending is_singleton_type so it also considers Final classes to be singletons should not be that difficult, but I will first check if this enum solution is workable for my project.

@gvanrossum
Copy link
Member

D'oh, I forgot you have to add Final when using a variable when I tried this myself.

The behavior with Enum members and is is clearly spelled out in PEP 586 ("Interactions with enums and exhaustiveness checks"), so I prefer sticking with that over non-standard cleverness with classes.

Can we close this issue?

@ilevkivskyi
Copy link
Member

ilevkivskyi commented Oct 7, 2019

Can we close this issue?

Initially I thought so, but I think ideas @Michael0x2a and @JukkaL proposed make sense:

  • Not requiring Final for a single-value enum is kind of convenient, and something one would naturally expect, note even you forgot to put it there.
  • Making type(x) is C equivalent to isinstance(x, C) if C is a final class is also kind of natural independently of this particular use case (actually mypyc already does this under the hood).
  • Finally, we should copy the example from either PEP 586 or PEP 484 (since they are essentially equivalent) to mypy docs, people often don't want read PEPs.

@jbwdevries
Copy link
Author

I can confirm the Enum trick works for my case.

I would also like to support @ilevkivskyi suggestions; option two feels a bit more natural than this one weird enum trick; updating the documentation would have let me find the enum trick sooner. At the moment, the documentation seems to hint to me that using is cannot be used.

Though I'm not sure about the suggestion regarding Final, since if the variable isn't Final, couldn't another part of the code change it, and therefore change the execution of the code?

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 8, 2019

if the variable isn't Final, couldn't another part of the code change it, and therefore change the execution of the code?

Since the enum has only one possible value, there is nothing else that can be assigned to it (that still matches the type).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants