-
Notifications
You must be signed in to change notification settings - Fork 114
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
Define laws for Traversable #373
Comments
I'd like to take this. |
I'm hitting a roadblock writing the naturality law for |
I'm no category theorist, so please excuse me if I say something nonsensical. Natural transformations are morphisms between functors, right? In that case we don't have them in ZIO Prelude (yet 😉). But let me ask "from the other side". Are we able to write some interesting laws/properties for Traversable with the current state of things? If yes, could you please open a PR with that? We'll figure something out about naturality in the meantime 👍 |
@zach-albia I think we don't need so Generic approach for generating Applicatives used by foreach as you do |
@Badmoonz your commit was super helpful in making the PR run in Dotty. Thanks! |
@zach-albia I think it will be nice to add some tests to check that laws are really working:
still cannot imagine such traversable instance that has valid Identity laws and broken Composition laws 🤔 |
So we now have 2 PRs implementing this
#449 has
So how do we do this guys? Do you @zach-albia and @Badmoonz coordinate with each other to create one PR? Or should we merge first one and then the other? In what order? |
@sideeffffect I integrated a lot of the code from #449 into #442. I think you can merge his first once #449 runs in 2.11, and then you can take a look at mine? I think mine needs some more review TBH. |
I don't want to reject all the job @zach-albia has done introducing new laws But if we want to keep things simple and valuable, really enough set of traversable laws would be
That's all we need!!! It's ever lesser law set than #449
I think we should introduce new laws in terms of existing such instances that work incorrectly but pass current laws |
If what @Badmoonz says is correct, than let's just have the one commutativity law. |
Yes I think we don't need five laws. The formulation I have seen normally has two, an identity law that calling |
For the record, here are papers discussing laws for Traversable:
Would somebody like to go forward and implement the identity law
? |
There have been some changes in the codebase, like renaming |
@sideeffffect I would love to have a go at it again as soon as I get the chance (which should be in a week or two). Will maintain @Badmoonz' insight on the minimum number of laws that need to be implemented. |
That said, if anyone reading this wants to take this instead before I get to chance to work on it, please go ahead. |
Difficulty: Advanced, requires familiarity with the concept of algebraic laws and testing them
Traversable
currently doesn't have any laws (only inherits those fromCovariant
). Figure out which laws should ZIO Prelude'sTraversable
abide. Then write those tests down inTraversable.laws
. Add tests for the missing instances inTraversableSpec
(tests for some basic instances, likeOption
orList
are already present).As an inspiration, have a look Cats or ScalaZ or Haskell Travers/able and their laws:
https://github.com/typelevel/cats/blob/master/laws/src/main/scala/cats/laws/TraverseLaws.scala#L9
https://github.com/scalaz/scalaz/blob/master/core/src/main/scala/scalaz/Traverse.scala#L162
https://hackage.haskell.org/package/base-4.14.0.0/docs/Data-Traversable.html#t:Traversable
The text was updated successfully, but these errors were encountered: