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

POC of unfoldable #1132

Closed
wants to merge 3 commits into from
Closed

POC of unfoldable #1132

wants to merge 3 commits into from

Conversation

julien-truffaut
Copy link
Contributor

Implement Unfoldable as of https://github.com/purescript/purescript-unfoldable #872

I don't know if it is a good idea but if Unfoldable extends Foldable then we can lift all methods from List[A] => List[A] to F[A] => F[A] e.g. prepend, append, filter, concat

This PR is just a proof of concept, I mainly wish to get some feedbacks.

@non
Copy link
Contributor

non commented Jun 15, 2016

Interesting!

I like the idea of something like this. I'm a bit worried that the laws are too strict. For example, Foldable doesn't require a specific ordering (so we can say that sets and similar are foldable). In principle it should be fine to construct a set through unfold but this would currently violate the laws (due to the order of the prepends not being maintained when someone later folds it back up).

Is this worth it? Foldable is somewhat lawless, and in some ways I'd be happy with an Unfoldable that was similar.

@codecov-io
Copy link

codecov-io commented Jun 15, 2016

Codecov Report

Merging #1132 into master will decrease coverage by 0.06%.
The diff coverage is 73.52%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1132      +/-   ##
=========================================
- Coverage   91.67%   91.6%   -0.07%     
=========================================
  Files         240     244       +4     
  Lines        3617    3671      +54     
  Branches       61      67       +6     
=========================================
+ Hits         3316    3363      +47     
- Misses        301     308       +7
Impacted Files Coverage Δ
laws/src/main/scala/cats/laws/FoldableLaws.scala 100% <ø> (ø) ⬆️
core/src/main/scala/cats/syntax/unfoldable.scala 0% <0%> (ø)
laws/src/main/scala/cats/laws/UnfoldableLaws.scala 100% <100%> (ø)
core/src/main/scala/cats/instances/list.scala 100% <100%> (ø) ⬆️
core/src/main/scala/cats/instances/set.scala 100% <100%> (ø) ⬆️
core/src/main/scala/cats/instances/vector.scala 97.82% <100%> (+0.39%) ⬆️
...n/scala/cats/laws/discipline/UnfoldableTests.scala 100% <100%> (ø)
core/src/main/scala/cats/Unfoldable.scala 37.5% <37.5%> (ø)
core/src/main/scala/cats/functor/Profunctor.scala 60% <0%> (-40%) ⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f90245d...81e522a. Read the comment docs.

@ceedubs
Copy link
Contributor

ceedubs commented Jun 16, 2016

@julien-truffaut neat! Do you have a specific use-case that prompted you to submit this? I'm always interested in how people are using things.

@non one possibility would be to start with these strict laws and later relax them if people find themselves wanting instances for types like Set. It's always easier to relax constraints than it is to add them.

@julien-truffaut
Copy link
Contributor Author

@ceedubs not really, I wanted to implement Unfoldable as in purescript but as @adelbertc and @non mentioned it is lawless. Then I wonder what could we do if we had both fold and unfold together and it seems that it is enough to implement all Seq / List functions

@julien-truffaut
Copy link
Contributor Author

What do you prefer:

  • a lawless Unfoldable with no dependency on Foldable
  • a stricter Unfoldable that guarantee the order is preserved with Foldable
  • something else?

@adelbertc
Copy link
Contributor

I recall Foldable doesn't really have laws and that the laws we have defined are a bit sketchy - is this a similar case? If so, I may be persuaded to include a standalone Unfoldable. Otherwise I prefer the second, to have it be consistent with Foldable.

@ceedubs ceedubs mentioned this pull request Aug 29, 2016
@julien-truffaut
Copy link
Contributor Author

Sorry for the inactivity on this PR. I am not sure how to go from this, I made Unfoldable lawless following the argument that we couldn't implement a Set instance if we maintain the order constraint between Foldable and Unfoldable. However, it seems wrong to have these two typeclasses without mutual laws as their methods are related.

All ideas are welcome! We can also simply close this PR until we have a clearer view, I mainly opened it to start a discussion.

@etorreborre
Copy link
Contributor

I am not sure about the laws of Unfoldable but today at work, someone asked me how to get all the consecutive values out of a State object and I thought I could use unfold for this.

@adelbertc
Copy link
Contributor

adelbertc commented Sep 23, 2016

Unfoldable looks interesting to me for similar reasons as @etorreborre pointed out.

That being said, if we go with Unfoldable[F] extends Foldable[F] we run into issues described here: #1379 (comment) with

def foo[F[_]: Unfoldable: Traverse] = ... // ambiguous Foldable[F]

new DefaultRuleSet(
name = "unfoldable",
parent = None,
// "noneConsistentWithDefault" -> forAll(() => laws.noneConsistentWithDefault[A]),
Copy link
Contributor

Choose a reason for hiding this comment

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

what's up here?

*/
def unfoldLeft[A, B](seed: B)(f: B => Option[(B, A)]): F[A]

def none[A]: F[A] =
Copy link
Contributor

Choose a reason for hiding this comment

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

def empty seems better to me. Do you foresee that conflicting with MonoidK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I would prefer to have unique function name for all cats typeclasses

def none[A]: F[A] =
Unfoldable.DefaultImpl.none(this)

def singleton[A](value: A): F[A] =
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will wind up being pure for Alternative instances.

* - If `f(b)` is `None`, then `unfoldLeft(b)(f)` should be empty.
* - If `f(b)` is `Some((b1, a))`, then `unfoldLeft(b)(f)` should consist of `a` appended by `unfoldLeft(b1)(f)`
*/
def unfoldLeft[A, B](seed: B)(f: B => Option[(B, A)]): F[A]
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like F[A] may be a functor. Certainly if I have B, B => Option[(B, A)] to get F[A] and if I have A => C then I can make F[C].

So, if for all F[A] there exists B, B => Option[(B, A)] to unfold to make F[A], then F[A] is a functor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if F is both a Foldable and Unfoldable then F is a Functor

Copy link
Contributor

@julienrf julienrf Nov 14, 2016

Choose a reason for hiding this comment

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

In some cases, it could be useful to return the B value at the end of the process, what do you think?

@typeclass trait Unfoldable[F[_]] { self =>

/**
* Build an F[A] from a `seed` and a generating function `f``
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be nice to have some stronger laws, even that might relate Unfoldable to Foldable or Functor.

Such as, if F[_]: Unfoldable and F[_]: Functor then,

unfoldLeft(seed)(fn).map(g) == unfoldLeft(seed)(fn.andThen { case (b, a) => (b, g(a)) })`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

case Nil => None
case x :: xs => Some((xs, x))
}
F.unfoldLeft(ga.reverse)(go)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we need to do the .reverse here? I don't see any laws about maintaining order -- is this just so that Unfoldable[List].fromList(xs) <-> xs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're right, I think it is an artifact from the impl where I made Unfoldable extends Foldable. I guess we could express an order law if F is also Foldable

@adamgfraser
Copy link

This looks really cool. One thought that struck me is that right now I think this can only cover linear structures like List or Stream as opposed to structures like Tree with another branching factor. Also it can't really support structures that are always nonempty because when unfold returns none to stop the unfolding there is no value to put in the last layer. Is it worth thinking about how to abstract over these?

Maybe something like S => (A, F[S]) for nonempty structures (like the way unfoldTree is implemented in Scalaz) and S => Option[(A, F[S])] for structures that could be empty? I think there is a connection to Free and Cofree here as well. This may be out of scope though.

@julien-truffaut
Copy link
Contributor Author

@adamgfraser really interesting idea, it would be great if Unfoldable could also generate Tree. This PR is really a proof of concept, I am not sure it is worth to merge as it is right now. Please feel free to contribute to this branch or create your own version!

@LukaJCB
Copy link
Member

LukaJCB commented Jan 24, 2018

I think we should revive this, especially given that we now require our Foldables to be orderered. Not sure if this PR can be salvaged as it's pretty out of date, but the main type class looks pretty good to me :)

@LukaJCB
Copy link
Member

LukaJCB commented Feb 5, 2018

So I think making Unfoldable lawless isn't a great idea.
If we have Unfoldable extend Foldable we can add some nice laws, as well as get a Functor instance for free, at that point we can also get a Traverse instance, so maybe we want Unfoldable extends Traverse? That would also solve the problem adelbert mentioned about ambiguous Foldable[F].

@kailuowang
Copy link
Contributor

Closing stale PRs. Feel free to reopen if there is interest to revive the effort.

@kailuowang kailuowang closed this Aug 5, 2019
@larsrh larsrh deleted the unfoldable branch April 11, 2020 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.