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 some new refolds. #27

Closed
wants to merge 5 commits into from
Closed

Add some new refolds. #27

wants to merge 5 commits into from

Conversation

sellout
Copy link
Contributor

@sellout sellout commented Jun 8, 2016

This adds ghyloM, dyna, codyna, codynaM refolds. As well as a
ganaM unfold.

FunctorT.translate has been removed, as it is identical to
FunctorT.transAna (and identical to .transCata, since the traversal
order is irrelevant) and attempting to add TraverseT.translateM made it clear
that sometimes you do care about whether its bottom-up or
top-down (because the constraints on F and G differ), so now always
explicitly use transAna or transCata.

Fixed the type parameter order for IdOps.ghylo.

Added a Nat type to matryoshka.fixedpoint.

Added type aliases for transforms, and a conversion from Transform to
Algebra.

Added optics for algebras and folds.

Made freeEqual available outside of tests.

This adds `ghyloM`, `dyna`, `codyna`, `codynaM` refolds. As well as a
`ganaM` unfold.

`FunctorT.translate` has been removed, as it is identical to
`FunctorT.transAna` (and identical to `.transCata`, since the traversal
order is irrelevant) and attempting to add `TraverseT.translateM` made it clear
that sometimes you do care about whether its bottom-up or
top-down (because the constraints on `F` and `G` differ), so now always
explicitly use `transAna` or `transCata`.

Fixed the type parameter order for `IdOps.ghylo`.

Added a `Nat` type to `matryoshka.fixedpoint`.

Added type aliases for transforms, and a conversion from Transform to
Algebra.

Added optics for algebras and folds.

Made `freeEqual` available outside of tests.
@sellout
Copy link
Contributor Author

sellout commented Jun 8, 2016

sigh once again scaladoc breaks the build.

@codecov-io
Copy link

codecov-io commented Jun 8, 2016

Current coverage is 76.55%

Merging #27 into master will increase coverage by 1.55%

  1. 2 files in .../matryoshka/patterns were modified. more
    • Misses -4
    • Hits +4
  2. 3 files in ...ain/scala/matryoshka were modified. more
    • Misses -5
    • Hits +2
  3. 3 files (not in diff) in .../matryoshka/patterns were modified. more
    • Misses -1
    • Hits +1
  4. 1 files (not in diff) in ...ain/scala/matryoshka were modified. more
    • Misses +1
    • Hits +3
@@             master        #27   diff @@
==========================================
  Files            25         25          
  Lines           416        465    +49   
  Methods         406        459    +53   
  Messages          0          0          
  Branches          8          5     -3   
==========================================
+ Hits            312        356    +44   
- Misses          104        109     +5   
  Partials          0          0          

Powered by Codecov. Last updated by 4c90b0e...60fd6fb

This triggered some other changes along the way, of course.
- added a dependency on scalaz-specs2 and got rid of our local equal
matcher
- fixed the `height` algebra to return something other than 0
equal(3)
}

// TODO: Should work with any Int, but stack overflows on big negatives.
Copy link
Contributor

Choose a reason for hiding this comment

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

In the implementation of unsafePerformSync, have you tried making it @tailrec by explicitly match-ing on the disjunction rather than fold-ing? I'm not sure if the Ops class gets in the way of scalac's "tco", but might work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you’re right that it should be @tailrec, but this actually overflows on the non-tailrecable mc91 (but feel free to tell me how I could make it tailrec).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, not sure how to make mc91 tailrec, maybe Trampoline to avoid overflow? (No idea if that is possible with Partial either!)

@wemrysi
Copy link
Contributor

wemrysi commented Jun 9, 2016

👍 LGTM.

A thought for the fixedpoint instances might be to make them value classes instead of aliases so that you can hang typeclass instances and operations off of them.

@@ -17,16 +17,16 @@
package matryoshka.specs2.scalacheck

import scala.{Any, StringContext}
import scala.Predef.Set
import scala.Predef.{Set}
Copy link
Contributor

Choose a reason for hiding this comment

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

Unecessary braces.

@sellout
Copy link
Contributor Author

sellout commented Jun 9, 2016

Yeah, there’s actually a PR (#13) that converts them to classes. I have to rebase that and fix whatever issues were still there.

- made `unsafePerformSync` tail recursive
- added `min` and `max` operations to `Nat`
- add `gapo` unfold
- add a bunch of docs
@sellout
Copy link
Contributor Author

sellout commented Jun 9, 2016

Ok, addressed your comments, and some other minor stuff (as always), if you want to take a look. Otherwise I’ll merge when the build passes.

@wemrysi
Copy link
Contributor

wemrysi commented Jun 9, 2016

Bravo on all the docs, merge away.

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