Add forany and foranyPar to Traversable #392 #424

Badmoonz · 2020-11-22T14:50:23Z

CLAassistant · 2020-11-22T14:50:29Z

All committers have signed the CLA.

Badmoonz · 2020-11-22T14:59:37Z

core/shared/src/main/scala/zio/prelude/Traversable.scala

@@ -122,6 +122,24 @@ trait Traversable[F[+_]] extends Covariant[F] {
      }
    }

+  def forany[G[+ _] : AssociativeEither : IdentityBoth : Covariant, A, B](fa: F[A])(f: A => G[B]): G[Option[B]] =


IdentityEither is useless for these case, we don't want to use IdentityEither.none in case of empty traversable^ because it 'none' behaves like failure
So i requested IdentityBoth for any ; may be there should be separate typeclass in hierarchy that produces G[Any]?

Badmoonz · 2020-11-22T15:07:03Z

I think we should give more attention for testing instances for effect types like ZIO, Future ...

FutureCommutativeEither for example seems broken

  Promise[Either[A, B]]().completeWith(fa.map(Left(_))).completeWith(fb.map(Right(_))).future

If any future fails faster than another one completes, whole result is failed, but by CommutativeEither laws it should select sucessful result;

ZIOCommutativeEither seems ok

sideeffffect

Awesome work, thank you @Badmoonz

sideeffffect · 2020-11-22T15:48:26Z

FutureCommutativeEither for example seems broken

Wow, thanks for finding this. Any help with fixing the instance would be much appreciated 🙇

sideeffffect · 2020-11-22T15:50:01Z

Also, as a stretch goal, do you think, you could implement analogous methods (forany1 and foranyPar1) on NonEmptyTraversable? 🙏 Those could return G[B] straight, without Option, since the collection is non-empty.

sideeffffect · 2020-11-22T16:48:27Z

I see there's a conflict. Could you please resolve it @Badmoonz ? 🙏

Badmoonz · 2020-11-22T17:47:45Z

I see there's a conflict. Could you please resolve it @Badmoonz ? 🙏

ok

sideeffffect · 2020-11-22T18:10:14Z

You will also need to run fix to make the linter happy 😸

Badmoonz · 2020-11-22T18:50:11Z

probably done :)

sideeffffect · 2020-11-22T18:50:47Z

Do you think you could also try the NonEmptyTraversable variants? 🙏
#424 (comment)

Badmoonz · 2020-11-22T19:14:20Z

Do you think you could also try the NonEmptyTraversable variants? 🙏
#424 (comment)

sure :)

within this issue( #392 )?

sideeffffect · 2020-11-22T20:08:54Z

core/shared/src/main/scala/zio/prelude/IdentityBoth.scala

+  /**
+   * The `IdentityBoth` instance for `ZIO`.
+   */
+  implicit def ZIOIdentityBoth[R, E]: IdentityBoth[({ type lambda[+a] = ZIO[R, E, a] })#lambda] =


Actually, could you do it the other way around? 🙏
The proper way is to have ZIOIdentityBoth in AssociativeBoth's companion object.

sideeffffect

Could you please move ZIOIdentityBoth?

justinhj · 2020-11-23T04:40:34Z

core/shared/src/main/scala/zio/prelude/AssociativeBoth.scala

@@ -1200,14 +1200,6 @@ object AssociativeBoth extends LawfulF.Invariant[AssociativeBothDeriveEqualInvar
      def both[A, B](fa: => Vector[A], fb: => Vector[B]): Vector[(A, B)] = fa.flatMap(a => fb.map(b => (a, b)))
    }

-  /**


Did you mean to delete this?

Yes, ZIOAssociativeBoth can be fully replaced by ZIOIdentityBoth. Or should we call it ZIOCommutativeIdentityBoth? But it needs to be in the AssociativeBoth companion object 😉
More detailed explanation here: https://meta.plasm.us/posts/2019/09/30/implicit-scope-and-cats/

adamgfraser · 2020-11-23T18:56:34Z

core/shared/src/main/scala/zio/prelude/Traversable.scala

+   * `AssociativeEither[G].both` operation,
+   * returning `None` if the collection is empty.
+   */
+  def forany[G[+_]: AssociativeEither: Covariant, A, B](fa: F[A])(f: A => G[B]): Option[G[B]] = {


Isn't this collectFirst? It applies the function f to each element in the structure and returns the first result that is a "success", where the meaning of success and failure is defined by the AssociativeEither instance?

Yes, that's what I was going for! Thank you @adamgfraser , I didn't know it could have this name 👍
https://superruzafa.github.io/visual-scala-reference/collectFirst/ 👈 TIL
Although I would suspect we might need IdentityEither for this.

I was pushed to make a proposal for it, when I was looking at the signature for foreach and thought there should be a dual for it -- hence the name forany.

@sideeffffect Wow! That Visual Scala resource is really cool!

I think we can get away with just having an AssociativeEither instance because None can serve as the default value if the structure is empty or if there is no successful value. However, I think we need to flip the order of the result types here so we return a G[Option[B]]. Otherwise we end up returning the last "failure" if the structure is non-empty and all of the results are failures.

Yes. Good point! But in that case, we would probably need IdentityBoth, to be able to create a G of None in case of an empty Traversable, right?
I've updated the ticket description #392
Please let me know, if you consider it accurate now 🙏

adamgfraser · 2020-11-23T18:57:00Z

core/shared/src/main/scala/zio/prelude/Traversable.scala

+   * `CommutativeEither[G].both` operation,
+   * returning `None` if the collection is empty.
+   */
+  def foranyPar[G[+_]: CommutativeEither: Covariant, A, B](fa: F[A])(f: A => G[B]): Option[G[B]] = {


collectFirstPar?

adamgfraser · 2020-11-23T18:57:29Z

core/shared/src/main/scala/zio/prelude/Traversable.scala

+   * Compute each element of the collection
+   * with specified effectual function `f`
+   * and then reduces the results using the
+   * `AssociativeEither[G].both` operation,


Suggested change

* `AssociativeEither[G].both` operation,

* `AssociativeEither[G].either` operation,

adamgfraser · 2020-11-23T18:57:45Z

core/shared/src/main/scala/zio/prelude/Traversable.scala

+   * Compute each element of the collection
+   * with specified effectual function `f`
+   * and then reduces the results using the
+   * `CommutativeEither[G].both` operation,


Suggested change

* `CommutativeEither[G].both` operation,

* `CommutativeEither[G].either` operation,

adamgfraser · 2020-11-23T18:59:25Z

core/shared/src/main/scala/zio/prelude/Assertions.scala

+  /**
+   * Failing assertion, used for unexpected cases
+   */
+  val unexpectedResult: Assertion[Any] =


There is an existing nothing assertion that you can use for this.

adamgfraser

Couple of suggestions on naming. Otherwise looks great! Thanks for all your work on this!

Badmoonz commented Nov 22, 2020

View reviewed changes

sideeffffect previously approved these changes Nov 22, 2020

View reviewed changes

Badmoonz dismissed sideeffffect’s stale review via 607561f November 22, 2020 18:49

Badmoonz force-pushed the feature-392-traversable-forany branch from 7d1cae7 to 607561f Compare November 22, 2020 18:49

Badmoonz force-pushed the feature-392-traversable-forany branch from 607561f to dc64475 Compare November 22, 2020 19:13

sideeffffect closed this Nov 22, 2020

sideeffffect reopened this Nov 22, 2020

Badmoonz force-pushed the feature-392-traversable-forany branch from dc64475 to a161092 Compare November 22, 2020 19:57

sideeffffect reviewed Nov 22, 2020

View reviewed changes

sideeffffect requested changes Nov 22, 2020

View reviewed changes

Add forany and foranyPar to Traversable zio#392

b4df7ea

Badmoonz force-pushed the feature-392-traversable-forany branch from 7fe3fda to b4df7ea Compare November 22, 2020 22:46

justinhj reviewed Nov 23, 2020

View reviewed changes

adamgfraser reviewed Nov 23, 2020

View reviewed changes

adamgfraser suggested changes Nov 23, 2020

View reviewed changes

adamgfraser mentioned this pull request Nov 24, 2020

Add forany1 and foranyPar1 to NonEmptyTraversable #427

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add forany and foranyPar to Traversable #392 #424

Add forany and foranyPar to Traversable #392 #424

Badmoonz commented Nov 22, 2020

CLAassistant commented Nov 22, 2020 •

edited

Loading

Badmoonz Nov 22, 2020

Badmoonz commented Nov 22, 2020

sideeffffect left a comment

sideeffffect commented Nov 22, 2020

sideeffffect commented Nov 22, 2020

sideeffffect commented Nov 22, 2020

Badmoonz commented Nov 22, 2020

sideeffffect commented Nov 22, 2020

Badmoonz commented Nov 22, 2020

sideeffffect commented Nov 22, 2020

Badmoonz commented Nov 22, 2020 •

edited

Loading

sideeffffect Nov 22, 2020

sideeffffect left a comment

justinhj Nov 23, 2020

sideeffffect Nov 23, 2020 •

edited

Loading

adamgfraser Nov 23, 2020

sideeffffect Nov 23, 2020 •

edited

Loading

adamgfraser Nov 24, 2020

sideeffffect Nov 24, 2020

adamgfraser Nov 23, 2020

adamgfraser Nov 23, 2020

adamgfraser Nov 23, 2020

adamgfraser Nov 23, 2020

adamgfraser left a comment

	* `AssociativeEither[G].both` operation,
	* `AssociativeEither[G].either` operation,

	* `CommutativeEither[G].both` operation,
	* `CommutativeEither[G].either` operation,

Add forany and foranyPar to Traversable #392 #424

Are you sure you want to change the base?

Add forany and foranyPar to Traversable #392 #424

Conversation

Badmoonz commented Nov 22, 2020

CLAassistant commented Nov 22, 2020 • edited Loading

Choose a reason for hiding this comment

Badmoonz commented Nov 22, 2020

sideeffffect left a comment

Choose a reason for hiding this comment

sideeffffect commented Nov 22, 2020

sideeffffect commented Nov 22, 2020

sideeffffect commented Nov 22, 2020

Badmoonz commented Nov 22, 2020

sideeffffect commented Nov 22, 2020

Badmoonz commented Nov 22, 2020

sideeffffect commented Nov 22, 2020

Badmoonz commented Nov 22, 2020 • edited Loading

Choose a reason for hiding this comment

sideeffffect left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sideeffffect Nov 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sideeffffect Nov 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamgfraser left a comment

Choose a reason for hiding this comment

CLAassistant commented Nov 22, 2020 •

edited

Loading

Badmoonz commented Nov 22, 2020 •

edited

Loading

sideeffffect Nov 23, 2020 •

edited

Loading

sideeffffect Nov 23, 2020 •

edited

Loading