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

Rust style force handling of return value #6936

Open
runfalk opened this issue Jun 4, 2019 · 24 comments
Open

Rust style force handling of return value #6936

runfalk opened this issue Jun 4, 2019 · 24 comments

Comments

@runfalk
Copy link

runfalk commented Jun 4, 2019

I tried to implement something like Rust's Result type. The interesting thing is that it has a linting attribute called #[must_use] which makes the linter enforce that the value is not ignored when used as a return type.

I think this could be very helpful when implementing functions that returns status and you want to ensure that it is not unhandled.

Consider the following toy implementation:

from typing import cast, Generic, Optional, TypeVar, Union
from whereever import MustUse

T = TypeVar("T")
E = TypeVar("E")

# We inherit from MustUse to ensure this type isn't ignore
class Result(Generic[T, E], MustUse):
    def __init__(self, value: Union[T, E], is_ok: bool):
        self._value = value
        self.is_ok = is_ok

    @classmethod
    def ok(cls, value: T) -> "Result[T, E]":
        return cls(value, is_ok=True)

    @classmethod
    def error(cls, value: E) -> "Result[T, E]":
        return cls(value, is_ok=True)

    @property
    def is_error(self) -> bool:
        return not self.is_ok

    def get_value(self) -> Optional[T]:
        if self.is_ok:
            return cast(T, self._value)
        return None

    def get_error(self) -> Optional[E]:
        if not self.is_ok:
            return cast(E, self._value)
        return None

def f(success: bool) -> Result[str, int]:
    if not success:
        return Result[str, int].error(123)
    return Result[str, int].ok("Hello")


# No warning is raised since the value is saved to a variable
a = f(True)

# No warning is raised since we convert the Result into an Optional
f(False).get_value()

# This will raise a warning since we never do anything with the Result like:
#     error: unused return value with attribute MustUse
# or
#     error: return value of function call must be used
f(True)
@ilevkivskyi
Copy link
Member

In principle I like this idea, although it is probably relatively low priority. If you want this to end up in typing, then you need to discuss it on typing tracker and/or typing-sig mailing list.

@JelleZijlstra
Copy link
Member

This is similar to #2499, although the proposed implementation mechanism here is different.

@bluetech
Copy link
Contributor

bluetech commented Jun 5, 2019

Not relevant for the issue, just wanted to say you can get closer to Rust with something like the following:

result.py
from typing import Generic, TypeVar, Union
from typing_extensions import Final, final


T = TypeVar('T')
E = TypeVar('E')

@final
class Ok(Generic[T]):
    def __init__(self, value: T) -> None:
        self.value: Final = value

    def is_error(self) -> bool:
        return False

@final
class Err(Generic[E]):
    def __init__(self, error: E) -> None:
        self.error: Final = error

    def is_error(self) -> bool:
        return True

Result = Union[Ok[T], Err[E]]

Example:

def divide(a: float, b: float) -> Result[float, ZeroDivisionError]:
    if b == 0.0:
        return Err(ZeroDivisionError())
    return Ok(a / b)


result = divide(10, 0)
print(result.is_error())
if isinstance(result, Ok):
    # reveal_type(result.value) -> float
    print(f'Ok({result.value}')
elif isinstance(result, Err):
    # reveal_type(result.error) -> ZeroDivisionError
    print(f'Err({result.error!r})')
else:
    # For exhaustiveness checking, see https://github.com/python/mypy/issues/5818.
    assert_never(result)

@ilevkivskyi
Copy link
Member

@JelleZijlstra Yeah, this may be one of the possible ways to move towards #2499

@JukkaL
Copy link
Collaborator

JukkaL commented Jun 6, 2019

Here is another random idea about handling errors using return values with more minimal syntax (at the cost of requiring some special-purpose primitives):

from mypy_extensions import Result, is_error
# Result would be magic alias almost like this:
#   T = TypeVar('T')
#   E = TypeVar('E', bound=Exception)
#   Result = Union[T, E]

def divide(a: float, b: float) -> Result[float, ZeroDivisionError]:
    if b == 0:
        return ZeroDivisionError()
    return a / b

def foo() -> Result[None, RuntimeError]:
    result = divide(10, 0)
    if is_error(result):
        # Type of result is 'ZeroDivisionError' here
        return RuntimeError()
    # Type of result is 'float' here
    print(result + 1)

is_error(x) would behave mostly like isinstance(x, Exception), but mypy would require that each Result return value is handled with is_error somewhere in the function body or returned (errors can't be just ignored).

In Python 3.8 foo could be written like this:

def foo() -> Result[None, RuntimeError]:
    if is_error(result := divide(10, 0)):
        return RuntimeError()
    # Type of result is 'float' here
    print(result + 1)

@bluetech
Copy link
Contributor

bluetech commented Jun 6, 2019

@JukkaL This would be neat. One problem that comes up is, what if T is itself an exception value? :)

@JukkaL
Copy link
Collaborator

JukkaL commented Jun 6, 2019

One problem that comes up is, what if T is itself an exception value? :)

It would have to be wrapped in another object, something like Result[X[Exception], RuntimeError], for some generic class X. Since this is rare, I like it better than wrapping all return values.

@refi64
Copy link
Contributor

refi64 commented Jun 7, 2019

One thing IMO worth noting, once you introduce monadic types you start having some oddities with e.g. callbacks, where the given and expected return types don't necessarily match, or a lot of utility unwrapping functions. Languages built around monads (e.g. Haskell) don't really have this type of issue because the style of the language itself blends with monads really well.

Maybe what Swift does would be worth a look? If what we really want is checked error handling that's not as verbose as Java's throws, Swift allows you to simply add throws and it infers the error types. Then, you have to use special syntax at the call site if you want the error to propagate up, in order to acknowledge its presence.

Of course I just have to plug my old error handling article at the same time. 😂 I think Swift largely did better than my idea did, but if we're really looking at return values it's not half bad.

@JukkaL
Copy link
Collaborator

JukkaL commented Jun 10, 2019

@refi64 I once thought about borrowing the error handling mechanism from Swift. I don't remember if I wrote anything about this.

Here is another idea inspired by Swift and @refi64's article:

from mypy_extensions import Error, check

def divide(a: float, b: float) -> float | Error:
    if b == 0:
        # Inferred to raise ZeroDivisionError due to explicit "raise"
        raise ZeroDivisionError()
    return a / b  # No exception inferred from this

def foo() -> None | Error:
    result = check(divide(10, 0))  # 'check' required to propagate exception
    print(result + 1)

def bar() -> None | Error:
    try:
        result = divide(10, 0)  # 'check' not required inside 'try'
    except ZeroDivisionError
        raise RuntimeError()
    print(result + 1)

def foobar() -> None:
    try:
        result = divide(10, 0)  # 'check' not required inside 'try'
    except ZeroDivisionError
        sys.exit(1)  # No checked exception from exit()
    print(result + 1)  # No checked exception from print()

# foo may raise checked ZeroDivisionError
# bar may raise checked RuntimeError
# foobar doesn't raise a checked exception

Random notes:

  • We'd use interprocedural inference of possible checked exception types.
  • Many built-in exceptions would unchecked by default (including ZeroDivisionError, IndexError, etc.). These would work as currently.
  • If a function may raise a checked exception, the return type would be t | Error where t is the normal return type. Error means that the function may raise some checked exception type(s).
  • To propagate a checked exception from an expression to the caller, place the expression inside check(...). At runtime, check(x) just returns the argument.
  • If a surrounding try statement catches all checked exceptions, no check(...) is required.
  • If all checked exceptions are caught inside function body, no Error return type is needed.
  • It would also be possible to explicitly annotate the exceptions using def f(...) -> t | Error[FooError, BarError].
  • Stubs could explicitly annotate some checked exceptions, but we'd probably be conservative to make migration easier. Maybe full use of checked exceptions would require the use of wrapper modules that are designed for checked exceptions.
  • check(main()) at module top level results in a traceback on error.
  • I don't really like the t | Error syntax.
  • Not sure if foo.read() or foo.write() should raise checked exceptions. print() probably shouldn't.
  • Each exception type such as RuntimeError can be used as checked or unchecked, depending on context. In user code each raise would be checked by default, but we'd probably have something like raise_unchecked(e) to raise an arbitrary unchecked exception.

@refi64
Copy link
Contributor

refi64 commented Jun 11, 2019

Honestly that looks amazing, though I feel like the semantics around this being, or at least looking like, a return value seem to be a bit confusing.

E.g.

def spam() -> int | Error:
    # ...

def eggs() -> None:
    # Is this valid
    x = spam()
    check(x)
    # but then how might this work?
    try:
        y = x
    # ...

Given the overall style of the proposal, maybe something simply like @raises might work a bit better?

As for the stuff like RuntimeError, maybe declare that only Exception subclasses are checked, not all Error subclasses? That way stuff like SystemExit and AssertionError don't make things weird.

@JukkaL
Copy link
Collaborator

JukkaL commented Jun 11, 2019

# Is this valid
x = spam()
check(x)

No, check(<expr>) would be needed if <expr> can raise an unchecked exception. It's the call spam() that can raise an exception, not reading the value of variable x.

Given the overall style of the proposal, maybe something simply like @raises might work a bit better?

Yeah, that is an option, but it wouldn't interact as nicely with Callable[...] types, in my opinion. For example, something like this could be supported (it may be too difficult to infer exception types automatically here):

E = TypeVar('E', bound=Exception)

def foo(f: Callable[[], None | Error[E]) -> None | Error[E]:
    check(f())

Not sure how we'd represent this using the decorator.

Also, in some real-world code the majority of functions might raise unchecked exceptions, and the @raises decorators could make the code look kind of noisy. I don't have a strong opinion on this, however. It would be useful to annotate some actual code using checked exceptions to get a better idea of how these would look in practice.

As for the stuff like RuntimeError, maybe declare that only Exception subclasses are checked, not all Error subclasses?

Most built-in exception classes, including RuntimeError, IndexError and AssertionError are derived from Exception, and it should be possible to treat at least some of these as unchecked. It may be reasonable that exceptions not derived from Exception cannot be raised as checked exceptions, but I'm not sure if this would actually be helpful. Somebody could plausibly want to treat SystemExit as a checked exception, even though it's not derived from Exception. I'd rather treat all exception types identically at least until we understand the practicalities better.

@refi64
Copy link
Contributor

refi64 commented Jun 12, 2019

Hmm true, I understand that, it just seems kinda odd that it looks like a return type but doesn't act one.

Most built-in exception classes...

Ugh right, this is what I get for writing comments on bug reports around midnight...

@remdragon
Copy link

remdragon commented Aug 13, 2019

I would be happy to volunteer to annotate a real project to test this. I don’t have any significant open source projects but I wouldn’t mind testing this on my internal code and sharing it with a few individuals under NDA as a test bed for this idea.

Personally I feel like all subclasses of Exception should be checked, as missing them leads to unexpected application errors. My experience with IndexError in particular is that I rarely want that to bubble out of a function because I lose sight of its meaning. I will usually convert it into a more meaningful exception if I need it to travel up further.

With regards to syntax, I like raises better than the pipe proposal even though it doesn’t jive as well with Callable. My proposal with Callable would be to add an optional 3rd element to it, like so:

FooFunc = Callable[[],None,Union[IndexError,IOError]]

I would even propose a step farther and propose something like the following for readability (I don’t know if Callable already supports kwargs):

FooFunc = Callable[
params=[],
returns=None,
raises=Union[IndexError,IOError]
]

As far as @raises goes, multiple types could be passed as *args but for a nod towards Callable maybe it would be better to require this:

@raises ( Union[IndexError,IOError] )
def foo() -> None:
pass

I had another idea about how we could solve this is create the ability to apply a Callable to a function definition, like so:

def foo(): # type: FooFunc
pass

This disadvantage here is it makes it harder to read a function’s signature in context but it’s less work to change function signatures. I’m not convinced this is a net win but I thought I’d throw it out there anyway.

[Edited because autocorrect sucks]

@remdragon
Copy link

Would someone please advise: How could I go about putting a bounty on this feature?

@ilevkivskyi
Copy link
Member

@remdragon

How could I go about putting a bounty on this feature?

I don't think we support anything like this, but maybe others know @gvanrossum @JukkaL

@gvanrossum
Copy link
Member

No, there are no bounties here.

@stereobutter
Copy link

I really like the monadic way or error handling as demonstrated by https://github.com/dry-python/returns. As someone else already noted this requires some machinery for wrapping, unwrapping and composing functions written in this style. The upside of having helper functions like map, bind, apply and friends for error handling is that it gives names to commonly occurring patterns of nested and/or branching control flow that are easier to reason about (once one is comfortable with the concept) than error handling via control flow statements.

@gvanrossum
Copy link
Member

Only a tiny number of people are going to adopt that style. It looks quite unpythonic to me.

@stereobutter
Copy link

stereobutter commented Oct 9, 2019

@gvanrossum I agree with your sentiment when applied to the python community as a whole; when only applied to the part of the community that uses mypy things might look different. The challenge is making a feature like this look and feel like python.

@gvanrossum
Copy link
Member

gvanrossum commented Oct 9, 2019 via email

@autoferrit
Copy link

autoferrit commented Sep 19, 2020

I believe this would be a great idea as someone who just wants to write safer code. I found the project https://github.com/dry-python/returns some time ago and loved the idea, but didn't use it because of the same reason @gvanrossum mentioned is that it's not very pythonic. And felt that those using my code and not familiar with that returns would just be confused. I would much rather implement something that mypy could work with.

My question as just an average developer is, until something like this is worked out in mypy or even a stand alone package, how would you guys recommend trying to at least start on something like this for a new code base? I like the syntax proposed in the comment #6936 (comment) with Ok and Err, but I agree having to unwrap the value in places wherever we need it, isn't super pretty. Maybe Ok for example could implement __str__, __dict__, etc or infer the type somehow so if we simple access the Ok("foo") it returns the value. That seems like it could help with having to use something.value anywhere.

I agree in a way that -> Result[str, Err]: isn't as clean as some other options but I like it more if it helps me write more stable code and it is pretty readable.

I guess my question is what could be a recommended pattern if I wanted to try to wrap my return values in one of the methods described here, or to have this style of error checking, in a way that might later worth with mypy. I know the method isn't decided yet but if done cleanly enough, migrating to the accepted pattern when/if mypy implements this or supports it, shouldn't be too bad with good testing in place. My favorite is the ability for a mypy setting when using these, to enforce that we always handle the Ok/Err whenever it is used.

Edit: Another library that is trying to do a Result type as well as Ok and Err
https://github.com/dbrgn/result/

@fritzo
Copy link

fritzo commented May 4, 2023

I just want to point out one use case is asyncio.create_task(). According to the docs, users must retain a reference to the return value to prevent the event loop from garbage collecting the task before completion. It would be great if those docs could be promoted to a static check.

Important Save a reference to the result of this function, to avoid a task disappearing mid-execution. The event loop only keeps weak references to tasks. A task that isn’t referenced elsewhere may get garbage collected at any time, even before it’s done. For reliable “fire-and-forget” background tasks, gather them in a collection:

@ethanhs
Copy link
Collaborator

ethanhs commented May 4, 2023

I'm not sure if something like must_use would really help with create_task, since we would only be able to check if the item returned by create_task is used in some way, not that a reference is maintained throughout the items lifetime. If you create a task, do one thing to it, then immediately delete all references to it, it is still used but may die before the task is completed.

@jgarvin
Copy link

jgarvin commented Jul 29, 2024

There are tons of functions that should work this way; in fact since error handling is typically done via exceptions in python requiring return values to be used should probably be the default for everything, it's almost always a bug in python when you ignore a return. The occasional thing that has a frequently unused return value should be manually opted out with some kind of annotation like MayNotUse[int] instead of MustUse[int].

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