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 Partition #1905 #1908

Merged
merged 28 commits into from
Aug 15, 2024
Merged

add Partition #1905 #1908

merged 28 commits into from
Aug 15, 2024

Conversation

RomanMIzulin
Copy link
Contributor

@RomanMIzulin RomanMIzulin commented Aug 8, 2024

I have added partition func to result module

Checklist

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

Related issues

🙏 Please, if you or your company finds dry-python valuable, help us sustain the project by sponsoring it transparently on https://github.com/sponsors/dry-python. As a thank you, your profile/company logo will be added to our main README which receives hundreds of unique visitors per day.

@RomanMIzulin
Copy link
Contributor Author

maybe add more tests

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

I would expect to have a partition function that:

  • Unwraps the values inside both Success and Failure objects

So, something like this:

vals = [Result.from_value(1), Result.from_failure('a')]
x: tuple[list[int], list[str]] = returns.methods.partition(vals)  # type is inferred automatically, of course
assert x[0] == [1]
assert x[1] == ['a']

the same for IOResult must be supported as:

vals = [IOResult.from_value(1), IOResult.from_failure('a')]
x: tuple[list[IO[int]], list[IO[str]]] = returns.methods.partition(vals)  # type is inferred automatically, of course
assert x[0] == [IO(1)]
assert x[1] == [IO('a')]

I guess that we can use ResultBased interface for that.

@RomanMIzulin
Copy link
Contributor Author

additional func witch do partition and unwraping?

@sobolevn
Copy link
Member

sobolevn commented Aug 8, 2024

not additional, but the only one. there's no problem in writing a for loop to partinion based on is_succesful

@RomanMIzulin
Copy link
Contributor Author

finally some working code

returns/result.py Outdated Show resolved Hide resolved
returns/result.py Outdated Show resolved Hide resolved
typesafety/test_result/test_partition.yml Outdated Show resolved Hide resolved
typesafety/test_result/test_partition.yml Outdated Show resolved Hide resolved
returns/result.py Outdated Show resolved Hide resolved
typesafety/test_result/test_partition.yml Outdated Show resolved Hide resolved
typesafety/test_result/test_partition.yml Outdated Show resolved Hide resolved
typesafety/test_result/test_partition.yml Outdated Show resolved Hide resolved
typesafety/test_result/test_partition.yml Outdated Show resolved Hide resolved
tests/test_result/test_result_partition.py Outdated Show resolved Hide resolved
@RomanMIzulin
Copy link
Contributor Author

can't understand why IOResult unwrapped to int instead of IO[int]

E   pytest_mypy_plugins.utils.TypecheckAssertionError: Invalid output:
E   Actual:
E     main:7: note: Revealed type is "Tuple[builtins.list[builtins.int], builtins.list[Any]]" (diff)
E   Expected:
E     main:7: note: Revealed type is "Tuple[builtins.list[IO[builtin.int]], builtins.list[IO[builtin.int]]" (diff)
E   Alignment of first line difference:
E     E: ... "Tuple[builtins.list[IO[builtin.int]], builtins.list[IO[builtin.int]...
E     A: ... "Tuple[builtins.list[builtins.int], builtins.list[Any]]"...

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Because we used an incorrect interface 🤦
Sorry!

We need this one: https://github.com/dry-python/returns/blob/master/returns/interfaces/specific/result.py#L71

@RomanMIzulin
Copy link
Contributor Author

Because we used an incorrect interface 🤦 Sorry!

We need this one: https://github.com/dry-python/returns/blob/master/returns/interfaces/specific/result.py#L71

used that, seems like working, got from unwrap_or_failure func
https://github.com/RomanMIzulin/returns/blob/7b37a0fd171bc6b4e8fb62daf97736cddc002617/returns/interfaces/unwrappable.py#L10

Copy link

codecov bot commented Aug 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #1908   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           80        81    +1     
  Lines         2485      2540   +55     
  Branches       437       447   +10     
=========================================
+ Hits          2485      2540   +55     

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

docs/pages/result.rst Outdated Show resolved Hide resolved
docs/pages/result.rst Outdated Show resolved Hide resolved
returns/methods/partition.py Outdated Show resolved Hide resolved
returns/methods/partition.py Outdated Show resolved Hide resolved
tests/test_result/test_result_methods.py Outdated Show resolved Hide resolved
tests/test_result/test_result_methods.py Outdated Show resolved Hide resolved
tests/test_result/test_result_methods.py Outdated Show resolved Hide resolved
docs/pages/methods.rst Outdated Show resolved Hide resolved
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Great work!

@sobolevn sobolevn merged commit e96a354 into dry-python:master Aug 15, 2024
23 checks passed
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.

feat: add partition func for result ojbects
3 participants