Add decorator to turn regular functions in Result returning ones #71

wbolster · 2021-10-31T18:51:13Z

Add decorator to turn regular functions in Result returning ones

Add a as_result() helper to make a decorator to turn a function into one
that returns a Result: Regular return values are turned into
Ok(return_value). Raised exceptions of the specified exception type(s)
are turned into Err(exc).

The decorator is signature-preserving, except for wrapping the return
type into a Result, of course.

For type annotations, this depends on typing.ParamSpec which requires
Python 3.10+ (or use typing_extensions); see
PEP612 (https://www.python.org/dev/peps/pep-0612/).

Fixes #33.

francium

Amazing. Very nice 👍

Don't forget a changelog entry

francium · 2021-11-01T04:12:49Z

For type checking, this depends on typing.ParamSpec which requires
Python 3.10+ (or use typing_extensions); see
PEP612 (https://www.python.org/dev/peps/pep-0612/)

What does this mean for Python <3.10 users? Ideally, we'd like to keep the library usable even if less functional/type checked

wbolster · 2021-11-01T20:15:15Z

What does this mean for Python <3.10 users? Ideally, we'd like to keep the library usable even if less functional/type checked

i've updated the pull request. the description / commit message has more information about this, and a similar note is in a comment in the code.

what do you think?

wbolster · 2021-11-01T20:18:16Z

update: see comment below; this is not entirely true at the moment

another problem is mypy: it runs in CI using different python versions, and mypy on different python versions has different support for python features, so that means it will not recognize some things.

in particular, mypy on some python versions needs ‘type: ignore’ overrides, while on the latest python version it must not have these overriding comments (because --strict warns about unused type overrides).

this is not fixable by adding more code.

i think the only way is to only run mypy on python 3.10 in CI? (or at least as a ‘required check’).

🤔

wbolster · 2021-11-01T21:02:39Z

ok, i got CI to pass ✅✅✅✅✅.

locally i was running an unreleased mypy version (to check out its partial ParamSpec support), which does not complain about the annotations, but does not check them either.

francium · 2021-11-03T04:33:00Z

Had a quick look, it looks good.
So what is the status of the ParamSpec support? Sorry, I'm not take familiar with mypy and Python typing to fully follow. The issue is just with CI, the end user will get proper expected type checking without any issues?

wbolster · 2021-11-03T10:40:49Z

cc @francium

the situation is better than i initially thought; see #71 (comment)

status is:

latest mypy does not know about ParamSpec, and will not check it. warnings can be suppressed with type: ignore comments, like this pull request does
callers of code annotated with ParamSpec will not give any typing errors, but the calls will not be type checked, i.e. similar to calling an untyped (or *args: Any, **kwargs: Any) function
(unreleased!) mypy development version has slightly better support wrt recognizing ParamSpec, but is still not able to actually check it. same behaviour for calling code.

so apart from docs/changelog, i think this code is fine to merge.

what about the name, is as_result() the best name? not sure myself.

francium · 2021-11-15T05:24:37Z

as_result name I think is fine. I can't think of anything better, so good enough :)

francium · 2021-11-15T05:25:55Z

So I understand what you've stated here as users can opt into using this decorator, but until mypy provides better typing, they'll have to accept untyped nature of this due to lack of ParamSpec support

result/result.py

francium · 2021-11-15T05:36:04Z

Looks good ~~otherwise~~, we can get this in 0.7.0
Just need changelog and readme.

wbolster · 2021-12-23T16:20:03Z

🥳 mypy now has improved (but incomplete) PEP612 support:

@as_result(ValueError)
def good(value: int) -> int:
    return value

@as_result(IndexError, ValueError)
def bad(value: int) -> int:
    raise ValueError

reveal_type(good)
reveal_type(bad)

mypy 0.910 complains a lot and makes everything Any:

$ mypy --version; mypy
mypy 0.910
src/result/result.py: note: In function "as_result":
src/result/result.py:316:16: error: The first argument to Callable must be a list of types or "..."  [misc]
src/result/result.py:337:22: error: The first argument to Callable must be a list of types or "..."  [misc]
src/result/result.py:343: error: Name "P.args" is not defined  [name-defined]
src/result/result.py:343: error: Name "P.kwargs" is not defined  [name-defined]
tests/test_result.py: note: In function "test_as_result":
tests/test_result.py:215:17: note: Revealed type is "Any"
tests/test_result.py:216:17: note: Revealed type is "Any"
Found 4 errors in 1 file (checked 3 source files)

mypy 0.921 complains a lot but does recognize the return types:

$ mypy --version; mypy
mypy 0.921
src/result/result.py: note: In function "as_result":
src/result/result.py:343: error: Name "P.args" is not defined  [name-defined]
src/result/result.py:343: error: Name "P.kwargs" is not defined  [name-defined]
tests/test_result.py: note: In function "test_as_result":
tests/test_result.py:215:17: note: Revealed type is "def (*Any, **Any) -> Union[result.result.Ok[builtins.int], result.result.Err[builtins.ValueError]]"
tests/test_result.py:216:17: note: Revealed type is "def (*Any, **Any) -> Union[result.result.Ok[builtins.int], result.result.Err[builtins.Exception]]"
Found 2 errors in 1 file (checked 3 source files)

mypy 0.930 is much happier:

$ mypy --version; mypy
mypy 0.930
tests/test_result.py: note: In function "test_as_result":
tests/test_result.py:215:17: note: Revealed type is "def (value: builtins.int) -> Union[result.result.Ok[builtins.int], result.result.Err[builtins.ValueError]]"
tests/test_result.py:216:17: note: Revealed type is "def (value: builtins.int) -> Union[result.result.Ok[builtins.int], result.result.Err[builtins.Exception]]"

codecov-commenter · 2021-12-23T16:26:56Z

Codecov Report

Merging #71 (27e3f55) into master (63c528c) will decrease coverage by 0.97%.
The diff coverage is 94.92%.

@@             Coverage Diff             @@
##            master      #71      +/-   ##
===========================================
- Coverage   100.00%   99.03%   -0.97%     
===========================================
  Files            3        3              
  Lines          252      308      +56     
===========================================
+ Hits           252      305      +53     
- Misses           0        3       +3

Impacted Files	Coverage Δ
tests/test_result.py	`98.86% <94.44%> (-1.14%)`	⬇️
src/result/result.py	`99.23% <95.45%> (-0.77%)`	⬇️
src/result/__init__.py	`100.00% <100.00%> (ø)`

wbolster · 2021-12-23T16:55:26Z

@francium pls have a look again. lots of type: ignore comments are not needed anymore.

and i even added docs

outdated

README.rst

src/result/result.py

francium

The only thing I think we still need to consider is mentioning publicly that mypy has a limitation regarding the type checking and users will currently have to accept Anys.
Do you think that should be mentioned in the README (FAQ section?) or in the CHANGELOG entry?

Add a as_result() helper to make a decorator to turn a function into one that returns a Result: Regular return values are turned into Ok(return_value). Raised exceptions of the specified exception type(s) are turned into Err(exc). The decorator is signature-preserving, except for wrapping the return type into a Result, of course. For type annotations, this depends on typing.ParamSpec which requires Python 3.10+ (or use typing_extensions); see PEP612 (https://www.python.org/dev/peps/pep-0612/). This is currently not fully supported by Mypy; see python/mypy#8645 Calling decorated functions works without errors from Mypy, but will not be type-safe, i.e. it will behave as if it is calling a function like f(*args: Any, **kwargs: Any) Fixes rustedpy#33.

wbolster · 2021-12-24T17:15:52Z

unless i'm missing something, this actually works well with mypy 0.390; see bottom of #71 (comment)

the incomplete mypy support (python/mypy#8645) seems to be about things that don't affect as_result(), such as parameter transformations etc, which as_result() does not do.

i've removed the misleading comments from the implementation; this was true until mypy 0.930 but not anymore.

francium · 2021-12-24T18:04:11Z

Sorry, I must've misunderstood. Great.

wbolster · 2021-12-24T18:23:03Z

fyi, i've updated the pr description and the (squashed) commit message to no longer refer to the behaviour of old mypy versions.

wbolster self-assigned this Oct 31, 2021

wbolster mentioned this pull request Oct 31, 2021

Add decorator to make plain functions return a Result #33

Closed

wbolster force-pushed the decorator-raise-to-result branch from 01c8a50 to 4faa486 Compare October 31, 2021 18:56

wbolster added the enhancement label Oct 31, 2021

francium previously requested changes Nov 1, 2021

View reviewed changes

wbolster mentioned this pull request Nov 1, 2021

PEP647 typing.TypeGuard based is_ok() and is_err() helpers #69

Closed

wbolster force-pushed the decorator-raise-to-result branch 4 times, most recently from 2ea0cf6 to cbb57c3 Compare November 1, 2021 20:00

wbolster requested a review from francium November 1, 2021 20:18

wbolster force-pushed the decorator-raise-to-result branch from cbb57c3 to 1f33ee5 Compare November 1, 2021 21:00

wbolster requested a review from dbrgn November 3, 2021 11:05

francium reviewed Nov 15, 2021

View reviewed changes

result/result.py Outdated Show resolved Hide resolved

wbolster force-pushed the decorator-raise-to-result branch from 1f33ee5 to 49040c1 Compare December 13, 2021 20:02

wbolster force-pushed the decorator-raise-to-result branch 2 times, most recently from 2c3aae2 to 3f04465 Compare December 23, 2021 16:12

wbolster force-pushed the decorator-raise-to-result branch from 3f04465 to ce6bc72 Compare December 23, 2021 16:25

wbolster changed the title ~~Add decorator to turn regular functions in Result returning ones~~ as_result decorator to turn regular functions into Result returning ones Dec 23, 2021

wbolster force-pushed the decorator-raise-to-result branch 3 times, most recently from 9c78f34 to bd61192 Compare December 23, 2021 16:52

wbolster requested a review from francium December 23, 2021 16:53

wbolster marked this pull request as ready for review December 23, 2021 16:53

wbolster force-pushed the decorator-raise-to-result branch from bd61192 to fd95812 Compare December 23, 2021 16:54

wbolster force-pushed the decorator-raise-to-result branch from fd95812 to 9d66177 Compare December 23, 2021 17:00

francium reviewed Dec 24, 2021

View reviewed changes

README.rst Show resolved Hide resolved

src/result/result.py Outdated Show resolved Hide resolved

wbolster force-pushed the decorator-raise-to-result branch from 9d66177 to 894296e Compare December 24, 2021 16:33

wbolster requested a review from francium December 24, 2021 16:35

francium approved these changes Dec 24, 2021

View reviewed changes

wbolster force-pushed the decorator-raise-to-result branch from 894296e to 27e3f55 Compare December 24, 2021 17:15

wbolster changed the title ~~as_result decorator to turn regular functions into Result returning ones~~ Add decorator to turn regular functions in Result returning ones Dec 24, 2021

wbolster merged commit ae2db8a into rustedpy:master Dec 24, 2021

wbolster deleted the decorator-raise-to-result branch December 24, 2021 18:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add decorator to turn regular functions in Result returning ones #71

Add decorator to turn regular functions in Result returning ones #71

wbolster commented Oct 31, 2021 •

edited

Loading

francium left a comment

francium commented Nov 1, 2021

wbolster commented Nov 1, 2021

wbolster commented Nov 1, 2021 •

edited

Loading

wbolster commented Nov 1, 2021

francium commented Nov 3, 2021

wbolster commented Nov 3, 2021 •

edited

Loading

francium commented Nov 15, 2021

francium commented Nov 15, 2021

francium commented Nov 15, 2021 •

edited

Loading

wbolster commented Dec 23, 2021

codecov-commenter commented Dec 23, 2021 •

edited

Loading

wbolster commented Dec 23, 2021

francium left a comment •

edited

Loading

wbolster commented Dec 24, 2021

francium commented Dec 24, 2021

wbolster commented Dec 24, 2021

Add decorator to turn regular functions in Result returning ones #71

Add decorator to turn regular functions in Result returning ones #71

Conversation

wbolster commented Oct 31, 2021 • edited Loading

francium left a comment

Choose a reason for hiding this comment

francium commented Nov 1, 2021

wbolster commented Nov 1, 2021

wbolster commented Nov 1, 2021 • edited Loading

wbolster commented Nov 1, 2021

francium commented Nov 3, 2021

wbolster commented Nov 3, 2021 • edited Loading

francium commented Nov 15, 2021

francium commented Nov 15, 2021

francium commented Nov 15, 2021 • edited Loading

wbolster commented Dec 23, 2021

codecov-commenter commented Dec 23, 2021 • edited Loading

Codecov Report

wbolster commented Dec 23, 2021

francium left a comment • edited Loading

Choose a reason for hiding this comment

wbolster commented Dec 24, 2021

francium commented Dec 24, 2021

wbolster commented Dec 24, 2021

wbolster commented Oct 31, 2021 •

edited

Loading

wbolster commented Nov 1, 2021 •

edited

Loading

wbolster commented Nov 3, 2021 •

edited

Loading

francium commented Nov 15, 2021 •

edited

Loading

codecov-commenter commented Dec 23, 2021 •

edited

Loading

francium left a comment •

edited

Loading