Closed mlw214 closed 3 years ago
Hi, this is currently by design. A Result effect is an exception free world so any called function within the effect needs to catch any exceptions and turn them into appropriate Error
values. This makes the effect pure in the sense that everything is simply functions, arguments and return values.
It also aligns with FSharpx.Extras:
open FSharpx.Result
[<EntryPoint>]
let main argv =
let xs = result {
failwith "this happened!"
return 42
}
match xs with
| Ok value -> printfn "Got value %A" value
| Error ex -> printfn "Got error: %A" ex
0 // return an integer exit code
Running this program gives:
➜ results dotnet run
Unhandled exception. System.Exception: this happened!
at Program.main(String[] argv) in /Users/dbrattli/Developer/results/Program.fs:line 10
This works the same as:
def test_result_effect_throws():
error = CustomException("this happend!")
@effect.result
def fn() -> Generator[int, int, int]:
_ = yield from Ok(42)
raise error
with pytest.raises(CustomException) as exc:
fn()
assert exc.value == error
Perhaps it's better to have another decorator for exception throwing functions that will turn the exception into an Error
object instead. So instead of:
def parse(string):
try:
value = float(string)
return Ok(value)
except Exception as exn:
return Error(exn)
We can write something like:
@result.raises(ValueError)
def parse(string):
value = float(string)
return Ok(value)
What do you think? It should perhaps return just value instead of Ok(value)
.
Ah, this makes sense. I think the decorator route is the way to go based on your explanation.
What do you think? It should perhaps return just value instead of Ok(value).
Are there any downsides by returning the value instead of Ok(value)
in the context of an effect.result
decorated function? I'm still very new to this library, so I don't know all the ins and outs. However, since it can return an Error
object, perhaps it makes more sense to have it return Ok(value)
just to be consistent?
Since this is something I want for a project I'm currently working on, I'll go ahead and take a shot at implementing the decorator approach. I can then extract it and submit a PR if you think it's worth adding. Do you have any specific thoughts on where this decorator should live (e.g., a specific module)?
I think it's ok to return the value directly and have a decorator that transforms values and exceptions to Ok
and Error
values. I just saw that return Ok
was not necessary after writing and then added the comment.
It's safe to add it to the expession.extra.result
folder, e.g misc.py
, utils.py
, ... Then we can move it into core
later if we feel it works really well with the library. Export it in __all__
and into __init__.py
of the extra
module.
PS: Look at curry
how it uses the functool.wraps
to preserve the name and the docstring of the decorated function.
@dbrattli I apologize for my slowness here. I've gotten around to this and took a stab at implementing a decorator with correct typing support. While the decorator logic is simple to implement, it unfortunately looks like getting full typing support can't quite be done (yet) - I detail the problems below. Note that I could totally be wrong, so I'd love your input. I'm currently calling the decorator catch
, but I'm 100% open to renaming it as I don't think it fully captures what it does. Here's the code:
from functools import wraps
from typing import Callable, Type, TypeVar, cast, overload
from expression.core import Error, Ok, Result
E = TypeVar("E", bound=Exception)
T = TypeVar("T")
@overload
def catch(
*, exception: Type[E]
) -> Callable[[Callable[..., T]], Callable[..., Result[T, E]]]:
...
@overload
def catch(f: Callable[..., T]) -> Callable[..., Result[T, Exception]]:
...
def catch(f: Callable[..., T] = None, *, exception=Exception):
def decorator(fn: Callable[..., T]) -> Callable[..., Result[T, E]]:
@wraps(fn)
def wrapper(*args, **kwargs) -> Result[T, E]:
try:
out = fn(*args, **kwargs)
except Exception as e:
return Error(cast(E, e))
else:
return Ok(out)
return wrapper
if f is not None:
return decorator(f)
else:
return decorator
@catch(exception=ZeroDivisionError)
def halved(a: float) -> float:
return a / 2 if a > 0 else a / 0
x = halved(2)
reveal_type(halved) # Revealed type is 'def (*Any, **Any) -> expression.core.result.Result[builtins.float*, builtins.ZeroDivisionError]'
reveal_type(x) # Revealed type is 'expression.core.result.Result[builtins.float*, builtins.ZeroDivisionError]'
I see two problems on the typing front. The biggest problem is that I don't believe there's a way to preserve the type information for the arguments with mypy. Pyre-check actually does have support for this (see here). Note that this problem is directly called out in python/mypy#3157. The second problem is being able to handle functions that can throw multiple types of exceptions. Currently, they'd have to all be captured with a parent class (e.g., Exception
). I believe this could be theoretically solved with something like Variadic Generics, though they aren't support currently (see python/typing#193).
I'd love to hear your thoughts/feedback on this before I make a PR.
This looks really nice and I also like the name catch
. I've kind of given up on mypy so Expression only uses pyright for type checking in CI (pylance for vscode) and with pyright the return type will be inferred correctly. The code looks great but is missing an isinstance
check for the error in case some other exception was thrown. Here is my version:
from functools import wraps
from typing import Any, Callable, Optional, Type, TypeVar, Union, cast, overload
from expression.core import Error, Ok, Result
TSource = TypeVar("TSource")
TError = TypeVar("TError", bound=Exception)
@overload
def catch(exception: Type[TError]) -> Callable[[Callable[..., TSource]], Callable[..., Result[TSource, TError]]]:
...
@overload
def catch(f: Callable[..., TSource], *, exception: Type[TError]) -> Callable[..., Result[TSource, TError]]:
...
def catch(
f: Optional[Callable[..., TSource]] = None, *, exception: Type[TError]
) -> Callable[[Callable[..., TSource]], Union[Callable[..., Result[TSource, TError]], Result[TSource, TError]]]:
def decorator(fn: Callable[..., TSource]) -> Callable[..., Result[TSource, TError]]:
@wraps(fn)
def wrapper(*args: Any, **kwargs: Any) -> Result[TSource, TError]:
try:
out = fn(*args, **kwargs)
except Exception as exn:
if isinstance(exn, exception):
return Error(cast("TError", exn))
raise
else:
ret: Result[TSource, TError] = Ok(out)
return ret
return wrapper
if f is not None:
return decorator(f)
return decorator
@catch(exception=ZeroDivisionError)
def halved(a: float) -> float:
return a / 2 if a > 0 else a / 0
x = halved(2)
reveal_type(halved)
reveal_type(x)
A way to allow multiple exception types could be handled if we allowed the decorators to be chained e.g:
from functools import wraps
from typing import Any, Callable, Optional, Type, TypeVar, Union, cast, overload
from expression.core import Error, Ok, Result
TSource = TypeVar("TSource")
TError = TypeVar("TError", bound=Exception)
TError_ = TypeVar("TError_", bound=Exception)
@overload
def catch(
exception: Type[TError_],
) -> Callable[
[Callable[..., Union[TSource, Result[TSource, TError]]]], Callable[..., Result[TSource, Union[TError, TError_]]]
]:
...
@overload
def catch(f: Callable[..., TSource], *, exception: Type[TError]) -> Callable[..., Result[TSource, TError]]:
...
def catch(
f: Optional[Callable[..., TSource]] = None, *, exception: Type[TError]
) -> Callable[[Callable[..., TSource]], Union[Callable[..., Result[TSource, TError]], Result[TSource, TError]]]:
def decorator(fn: Callable[..., TSource]) -> Callable[..., Result[TSource, TError]]:
@wraps(fn)
def wrapper(*args: Any, **kwargs: Any) -> Result[TSource, TError]:
try:
out = fn(*args, **kwargs)
except Exception as exn:
if isinstance(exn, exception):
return Error(cast("TError", exn))
raise
else:
if isinstance(out, Result):
return cast(Result[TSource, TError], out)
return Ok(out)
return wrapper
if f is not None:
return decorator(f)
return decorator
@catch(exception=ArithmeticError)
@catch(exception=ZeroDivisionError)
def halved(a: float) -> float:
return a / 2 if a > 0 else a / 0
reveal_type(halved)
for x in halved(0):
reveal_type(x)
print(x)
This is great! Thanks for your corrections/input.
One thing I noticed is that the decorator doesn't appear to work well with the @curried
decorator. Personally, I'd like to get them to play nicely together, if possible. I'll play around with @catch
to see if I can make that happen. Regardless, I hope to have a PR for you this weekend.
@mlw214 I noticed you referenced python/typing#193 on variadic generics in this thread. Heads up that we've been working on a draft of a PEP for this in PEP 646. If this is something you still care about, take a read and let us know any feedback in this thread in typing-sig. Thanks!
Currently, it looks like any non-Expression exceptions thrown in a
effect.result
decorated function is re-raised by the decorator. I'm wondering if it might be useful to have the decorator return an Error object instead, wrapping the raised Exception object?This would reduce the amount of manual conversions from raised exceptions to Error objects, enabling easier interfacing with code/libraries that throw exceptions.
Thoughts? Happy to take a stab at implementing this. Either way, I'm digging this library!