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 bio parallel syntax #1041

Merged
merged 8 commits into from
May 7, 2020
Merged

Conversation

Adriani277
Copy link
Contributor

closes #1034

@Adriani277 Adriani277 force-pushed the add-bio-parallel-syntax branch from ad6950b to dedfd27 Compare May 6, 2020 11:20
@Adriani277 Adriani277 force-pushed the add-bio-parallel-syntax branch from dedfd27 to 11aced3 Compare May 6, 2020 11:22
Copy link
Member

@neko-kai neko-kai left a comment

Choose a reason for hiding this comment

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

@Adriani277 Thank you! 🙏 Everything is good, except I'd rather avoid symbolic aliases entirely. An exception was made for *> & <*(but e.g. not for >>=) due to them being exceedingly common, but parallel combinators are comparatively much rarer.

@Adriani277
Copy link
Contributor Author

@neko-kai No problem, will remove them

@Adriani277 Adriani277 requested a review from neko-kai May 6, 2020 16:53
x[zio.ZIO](zio.UIO.succeed(()), zio.UIO.succeed(()))
}

"BIOAsync.race is callable" in {
Copy link
Member

Choose a reason for hiding this comment

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

@Adriani277 BIOParallel ops should also be available when only BIOAsync is visible, the easiest way to do it would be to just copypaste them down into BIOAsync(3)Ops (as is done for BIOMonad ops)

@@ -1,12 +1,52 @@
package izumi.fundamentals.bio.test

import izumi.functional.bio.{BIO, BIOArrow, BIOAsk, BIOBifunctor3, BIOFork, BIOFork3, BIOFunctor, BIOLocal, BIOMonad, BIOMonad3, BIOMonadAsk, BIOMonadError, BIOPrimitives, BIOPrimitives3, BIOProfunctor, BIOTemporal, BIOTemporal3, F}
import izumi.functional.bio.{BIO, BIOArrow, BIOAsk, BIOAsync, BIOBifunctor3, BIOFork, BIOFork3, BIOFunctor, BIOLocal, BIOMonad, BIOMonad3, BIOMonadAsk, BIOMonadError, BIOParallel, BIOPrimitives, BIOPrimitives3, BIOProfunctor, BIOTemporal, BIOTemporal3, F}
Copy link
Member

Choose a reason for hiding this comment

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

you may want to move the imports for BIOAsync/BIOParallel down into their respective tests, since they impact the visibility of operators and may result in false positive results

@Adriani277 Adriani277 requested a review from neko-kai May 6, 2020 17:57
import izumi.functional.bio.BIOAsync3

def x[F[-_, +_, +_]: BIOAsync3](a: F[Any, Nothing, Unit], b: F[Any, Nothing, Unit]) = {
a zipPar b
Copy link
Member

Choose a reason for hiding this comment

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

@Adriani277 This keeps working because of the BIOParallel import above, with it cleaned up it fails.

Screenshot 2020-05-06 at 19 00 02

Screenshot 2020-05-06 at 19 01 03

Copy link
Contributor Author

@Adriani277 Adriani277 May 6, 2020

Choose a reason for hiding this comment

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

Ah apologies, seems I missed that BIOParallel import. BIOSync should have been working without the BIOParallel import. I will try to investigate why this is not working
@neko-kai The only reason I can see for it not working is due to BIOAsyncOps not extending BIOParallelOps but I am not sure how to get that to work since BIOAsyncOps already extends BIOOps.
Any tips?

Managed to get it working by making BIOParallel a trait.
Will push the changes tomorrow morning

@Adriani277 Adriani277 requested a review from neko-kai May 7, 2020 08:07
Copy link
Member

@neko-kai neko-kai left a comment

Choose a reason for hiding this comment

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

@Adriani277 Thank you! 🙏

@neko-kai neko-kai merged commit 793003f into 7mind:develop May 7, 2020
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.

Add Cats Parallel
2 participants