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

Add exceptions to @future_safe; deprecate FutureResultE #1880

Merged
merged 11 commits into from
Jul 27, 2024

Conversation

RomanMIzulin
Copy link
Contributor

I have added exceptions to @future_safe; removed FutureResultE

Checklist

  • [x ] I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
  • [x ] I have created at least one test case for the changes I have made
  • [x ] I have updated the documentation for the changes I have made
  • [x ] I have added my changes to the CHANGELOG.md

Related issues

@RomanMIzulin
Copy link
Contributor Author

RomanMIzulin commented Jul 21, 2024

not sure about quality of PR and deleting FutureResultE, but project is in beta, kinda breaking changes are allowed I guess

CHANGELOG.md Outdated
@@ -10,6 +10,8 @@ See [0Ver](https://0ver.org/).

### Features

- Add picky exceptions to `future_safe` decorator like `safe` has.
- Remove FutureResultE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, don't remove it just yet. Let's first provide an alternative. Moreover, maintaining an alias is simple enough.

Comment on lines 61 to 63
future_instance = _coro_three('0')

await future_instance
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
future_instance = _coro_three('0')
await future_instance
await _coro_three('0')

future_instance = _coro_two(0)

assert isinstance(future_instance, FutureResult)
assert isinstance(await future_instance, IOFailure)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to check that IOFailure has an expected error.

) -> int:
return 1

reveal_type(test) # N: Revealed type is "def (first: builtins.int, second: Union[builtins.str, None] =, *, kw: builtins.bool =) -> returns.future.FutureResult[builtins.int, builtins.Exception]"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be:

Suggested change
reveal_type(test) # N: Revealed type is "def (first: builtins.int, second: Union[builtins.str, None] =, *, kw: builtins.bool =) -> returns.future.FutureResult[builtins.int, builtins.Exception]"
reveal_type(test) # N: Revealed type is "def (first: builtins.int, second: Union[builtins.str, None] =, *, kw: builtins.bool =) -> returns.future.FutureResult[builtins.int, builtins.ValueError]"

) -> int:
return 1

reveal_type(test) # N: Revealed type is "def (first: builtins.int, second: Union[builtins.str, None] =, *, kw: builtins.bool =) -> returns.future.FutureResult[builtins.int, builtins.Exception]"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
reveal_type(test) # N: Revealed type is "def (first: builtins.int, second: Union[builtins.str, None] =, *, kw: builtins.bool =) -> returns.future.FutureResult[builtins.int, builtins.Exception]"
reveal_type(test) # N: Revealed type is "def (first: builtins.int, second: Union[builtins.str, None] =, *, kw: builtins.bool =) -> returns.future.FutureResult[builtins.int, builtins.ValueError]"

@sobolevn
Copy link
Member

I will help you with that! Thanks!

@sobolevn sobolevn changed the title WIP: add exceptions to @future_safe; removed FutureResultE Add exceptions to @future_safe; removed FutureResultE Jul 27, 2024
@sobolevn sobolevn marked this pull request as ready for review July 27, 2024 10:49
@sobolevn sobolevn changed the title Add exceptions to @future_safe; removed FutureResultE Add exceptions to @future_safe; deprecate FutureResultE Jul 27, 2024
Copy link

codecov bot commented Jul 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (82ef3ef) to head (b613e53).
Report is 105 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #1880   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           80        80           
  Lines         2485      2523   +38     
  Branches       437       446    +9     
=========================================
+ Hits          2485      2523   +38     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sobolevn sobolevn merged commit a82b7f5 into dry-python:master Jul 27, 2024
23 checks passed
@sobolevn
Copy link
Member

Thanks a lot!

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

Successfully merging this pull request may close these issues.

2 participants