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

Map async #162

Closed
wants to merge 2 commits into from
Closed

Map async #162

wants to merge 2 commits into from

Conversation

JLessinger
Copy link
Contributor

like map, but async. Similar to the PR that added and_then_async.

@codecov-commenter
Copy link

Codecov Report

Merging #162 (d52a922) into master (730ef99) will increase coverage by 0.11%.
The diff coverage is 98.20%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #162      +/-   ##
==========================================
+ Coverage   97.41%   97.52%   +0.11%     
==========================================
  Files           4        4              
  Lines         464      565     +101     
==========================================
+ Hits          452      551      +99     
- Misses         12       14       +2     
Files Coverage Δ
src/result/__init__.py 100.00% <ø> (ø)
src/result/result.py 95.10% <100.00%> (+0.42%) ⬆️
tests/test_result.py 99.22% <100.00%> (+0.04%) ⬆️
tests/test_result_do.py 98.00% <97.53%> (-2.00%) ⬇️

Copy link
Member

@francium francium left a comment

Choose a reason for hiding this comment

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

Looks good, but we'll need to rebase the commit on top of other do_async PR after it's merged

@francium
Copy link
Member

There's a few other methods that accepts a Callable. Should we just add async variants for them all in a single (this) PR? Rather than piece by piece do them?

Also I guess this would create a divide between async functions and non-async functions without a possible way to merge the two worlds? Function coloring problem?

Does this mean we'd forever have to have a _async variant for all methods that accept a Callable for which we want to support an async variant?

@JLessinger
Copy link
Contributor Author

I agree it makes sense to do this in one batch, but I think we should get this one shipped since it's ready. The rest of the methods kind of seem like long-tail usage anyway. map and and_then hit maybe 99% for me.

Interesting read - need to look into how Go does async.

But yes, I think if you have async, it has to be separate forever. We've basically extended this library to provide not only the Result monad, but also a second composite async-Result monad.

async/await is just the do-notation of the Promise monad

https://gist.github.com/peter-leonov/c86720d1517235a1f28cd453a9d39bb4

@francium
Copy link
Member

francium commented Dec 20, 2023

Some languages don't suffer from the function coloring problem, Zig if I recall and Go like you mention. I don't know if there's any possible solution in Python land sadly.

Makes sense, we can defer the others in a separate PR.

Let me know when you've resolved some of the conflicts, I'll review then.

@francium
Copy link
Member

francium commented Dec 20, 2023

async/await is just the do-notation of the Promise monad

hmm, might be really a stretch, but would it be possible to implement do-notation for a Result monad with async/await syntax? I'm not sure what it would look like...and it may be quite confusing possibly.

An awaitable object is an object that defines await() method returning an iterator

What if Ok and Err implemented __await___? Would that even make sense? Or would you need to create a separate Do object that implemented __await__.

Just a random idea since the generator based do approach didn't work out due to typing issues/limitations.


Also found this, not sure if it's at all applicable, I haven't really understood these things to fully comprehend the magic that's involved in monads or effects -- https://garciat.com/posts/python-algebraic-effects-async-await

@francium francium mentioned this pull request Dec 23, 2023
francium added a commit that referenced this pull request Dec 23, 2023
Continuation of #162
@francium
Copy link
Member

Resolved the merge conflicts and merged in #165

@francium francium closed this Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants