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

Expose a cast variant to remove Optional #645

Closed
cjerdonek opened this issue Jun 11, 2019 · 12 comments
Closed

Expose a cast variant to remove Optional #645

cjerdonek opened this issue Jun 11, 2019 · 12 comments

Comments

@cjerdonek
Copy link
Member

I recently noticed that it would be useful to have a cast() variant that removes Optional (e.g. for DRY purposes). This would be useful in cases where earlier code has ruled out the None case and e.g. mypy's type inference isn't able to detect it. I believe this could also help prevent errors because as it is people can pass the wrong typ into cast() (or code can change over time, etc).

This is related to (or a special case of) issue #565. My apologies in advance if this feature already exists or if the idea has already been rejected.

@gvanrossum
Copy link
Member

I don't think it exists. Let me see if I understand you correctly.

def is_null(arg: object) -> bool:
    # A silly function that dynamically checks for null and empty values
    return not arg

class C:
    # Some class with some method
    def meth(self) -> int: ...

def foo(arg: Optional[C]) -> int:
    if is_null(arg):
        return 0
    return arg.meth()  # error: Item "None" of "Optional[C]" has no attribute "meth

The error on the last line is a false positive since is_null(arg) checks for None.

So you would want to write something like

    arg = cast_away_optional(arg)
    reveal_type(arg)  # Should print C, not Optional[C]
    return arg.meth()  # Should not be an error

Am I right?

@cjerdonek
Copy link
Member Author

Yep, that's right!

@JelleZijlstra
Copy link
Member

It's not quite the same, but mypy already supports doing this with assertions:

$ mypy -c '''                          
from typing import Optional
def f(x: Optional[int]) -> None:
    reveal_type(x)
    assert x is not None
    reveal_type(x)
'''
<string>:4: error: Revealed type is 'Union[builtins.int, None]'
<string>:6: error: Revealed type is 'builtins.int'

Of course, unlike the proposed cast operation, this also does a runtime check.

@gvanrossum
Copy link
Member

Likely the runtime overhead of a single is None comparison is less than that of a single no-op function call. So it sounds like the assert is the better solution. @cjerdonek Do you agree?

@cjerdonek
Copy link
Member Author

cjerdonek commented Jun 11, 2019

The assert is good to know (I didn't know about that when I filed the issue), and I agree is good for those cases. However, there is at least one case where it seems like it might not be as good (and was one of the reasons that led me to filing this). Take this snippet from pip's code base:

for line in data.split(b'\n')[:2]:
    if line[0:1] == b'#' and ENCODING_RE.search(line):
        encoding = ENCODING_RE.search(line).groups()[0].decode('ascii')
        return data.decode(encoding)

(from https://github.com/pypa/pip/blob/53d57a2accf994dccb5aee563368fa2ac8f55fa2/src/pip/_internal/utils/encoding.py#L33-L36 )

Here the issue is that after the if check we know that the return value of the method call (ENCODING_RE.search(line)) won't be None. A cast function like I suggested would let one do the assignment inline, like so:

encoding = cast_away_optional(
  ENCODING_RE.search(line),
).groups()[0].decode('ascii')

It seems using an assert would require adding more.

@gvanrossum
Copy link
Member

Okay, though the desire to do this in-line rather than using a separate statement makes the desire a lot weaker. It's a slippery slope; Python has inline conditionals, loops (comprehensions) and now (in 3.8) assignments, but not other things like try/except, while, del, or, indeed, assert...

@cjerdonek
Copy link
Member Author

but not other things like try/except, while, del, or, indeed, assert...

Indeed.. Really, the suggestion is just to add a light-weight way to tell the type-checker that something isn't None (which doesn't automatically require using assert). Using cast() as is is kind of annoying because e.g. the caller shouldn't have to know that ENCODING_RE.search() returns Match[bytes]:

encoding = cast(
    Match[bytes],
    ENCODING_RE.search(line),
).groups()[0].decode('ascii')

And using assert requires adding two lines including an assignment (an assignment expression can't be used because pip needs to support older versions):

for line in data.split(b'\n')[:2]:
    if line[0:1] == b'#' and ENCODING_RE.search(line):
        match = ENCODING_RE.search(line)
        assert match is not None
        encoding = match.groups()[0].decode('ascii')
        return data.decode(encoding)

But if you want to close this, I won't feel bad because I had a chance to make my case. :)

@gvanrossum
Copy link
Member

I guess you can write your own:

T = TypeVar('T')
def cast_away_optional(arg: Optional[T]) -> T:
    assert arg is not None
    return arg

(Untested.)

@cjerdonek
Copy link
Member Author

Thanks. Good to know.

@dkgi
Copy link

dkgi commented Jun 12, 2019

FWIW we use something very similar internally:
https://github.com/facebook/pyre-check/blob/master/pyre_extensions/__init__.py#L7-L8

@dpinol
Copy link

dpinol commented Nov 28, 2023

I guess you can write your own:

T = TypeVar('T')
def cast_away_optional(arg: Optional[T]) -> T:
    assert arg is not None
    return arg

(Untested.)

Unfortunately this solution is not useful for instance for static usages, eg to define a TypeGuard where the TypeVar may contain None.

@sliedes
Copy link

sliedes commented Apr 17, 2024

I think this would be quite useful. I have code like this:

class Node:
    seq_no: int

class Subscriber:
    current: Node | None

subscribers: list[Subscriber]

subs = [sub for sub in subscribers if sub.current is not None]
subs.sort(key=lambda sub: sub.current.seq_no)

If I wanted to use an assert, I'd need to turn the lambda into a named function with the assert. I can use cast(Node, sub.current).seq_no, but in my case the type of Node is again more complex (and generic).

Also, and of this I do not know if this is a mypy bug (I'll raise it there if you ask me to!) or something about how sort is typed or something else, while I can write a cast_not_none, it doesn't seem to work for the sorting key:

https://mypy-play.net/?mypy=latest&python=3.12&gist=e2eac81d84a965cafaeced5eecb40175

from typing import TypeVar

class Node:
    seq_no: int

class Subscriber:
    current: Node | None

T = TypeVar("T")

def cast_not_none(x: T | None) -> T:
    assert x is not None
    return x

subscribers: list[Subscriber]

subs = [sub for sub in subscribers if sub.current is not None]
subs.sort(key=lambda sub: cast_not_none(sub.current).seq_no)  # error: Never has no attribute "seq_no"

reveal_type(subscribers[0].current)  # --> Node | None
reveal_type(cast_not_none(subscribers[0].current))  # --> Node
(lambda sub: reveal_type(sub.current))(subscribers[0])  # --> Node | None
(lambda sub: reveal_type(cast_not_none(sub.current)))(subscribers[0])  # --> Node
subs.sort(key=lambda sub: reveal_type(cast_not_none(sub.current)))  # --> Never

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

No branches or pull requests

6 participants