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

Upgrade Cats dependency to 0.7 #212

Closed
alexandru opened this Issue Aug 22, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@alexandru
Copy link
Member

commented Aug 22, 2016

Cats 0.7 is out and dependency should be upgraded.

@lukestephenson

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2016

Note that monix 2.0-RC11 did not work with cats 0.7. There are breaking changes.

[error] (run-main-0) java.lang.AbstractMethodError: monix.cats.EvaluableInstances$$anon$1.tailRecM(Ljava/lang/Object;Lscala/Function1;)Ljava/lang/Object;
java.lang.AbstractMethodError: monix.cats.EvaluableInstances$$anon$1.tailRecM(Ljava/lang/Object;Lscala/Function1;)Ljava/lang/Object;
    at cats.free.Free.foldMap(Free.scala:131)
    at cats.free.Free.foldMapUnsafe(Free.scala:142)
    at demo.AppInterpreter.run(AppInterpreter.scala:12)
    at app.Main$$anonfun$main$1.apply$mcV$sp(Main.scala:20)
    at app.Main$.timed(Main.scala:26)
    at app.Main$.main(Main.scala:17)
    at app.Main.main(Main.scala)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)

In my example, I was using

script.foldMapUnsafe(xxx)

so I was surprised to see cats trying to call tailRecM on the monix cats Monad. Could be a bug in cats.

lukestephenson pushed a commit to lukestephenson/monix that referenced this issue Aug 24, 2016

luke.stephenson
Upgrade monix to cats 0.7.0
Still a WIP
- Some law tests are broken as a result of this change.
- Assuming all monix monads are trampolined
- Need to add tests to verify monix monads are trampolined and stack safe

See monix#212
@lukestephenson

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2016

I'm now getting some test failures. To resolve this I'm looking to supply custom equality methods for the checking (it's comparing two Task[Task[Int]] and only executing the outer task (unless for the logic to be correct both should be Suspend or BindSuspend).

[info] - CoflatMap[Task[Int]].coflatMap.coflatten throughMap *** FAILED ***
[info]   Falsified after 0 passed tests. (Checkers.scala:36)
[info]   > Labels of failing property:
[info]   (Suspend(<function0>) ?== BindSuspend(<function0>,<function1>)) failed
[info]   > ARG_0: Suspend(<function0>)
[info]     minitest.laws.Checkers$class.check(Checkers.scala:36)
[info]     monix.cats.TaskLawsSuite$.check(TaskLawsSuite.scala:26)

Hopefully I can submit a PR soon.

@alexandru

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2016

Unfortunately I'm blocked by this issue in Cats. ATM FlatMap / Monad instances are no longer implementable for Observable because of tailRecM: typelevel/cats#1329

@alexandru

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2016

OK, I managed to reconfigure ScalaCheck, such that the tailRecM tests pass for Observable. I'm using the following parameters:

    Parameters.default
      .withMinSuccessfulTests(10)
      .withMaxDiscardRatio(50.0f)
      .withMaxSize(6)

Not good though, because it weakens the tests. I hope Cats will provide a solution for monads that do not want to implement tailRecM in the future.

alexandru added a commit that referenced this issue Aug 25, 2016

Upgrade cats (issue #212), optimize Task, refactorings (#214)
- upgrade the Cats dependency to version 0.7.0, see issue #212 
- rename `eval` to `evalAlways` across the board (in `Task`, `Coeval` and `Observable`), but keep `evalAlways` with the `@deprecated` sign
- rename `eval(Coeval)` to `coeval(Coeval)` in `Task` and `Observable`
- refactor the `Task` internals again, for optimizations
  - simplifies the internal states, e.g. instead of having `Now`, `Error`, `Always` and `Once`, we now have a single `Delay(coeval)`
  - get rid of `Task.Attempt`, it never made any sense that one. People can use `Coeval.Attempt` if they need a `Try` alternative (and convert to `Task` if they end up needing a `Task`)
- moved everything from `monix.types.shims` to `monix.types`
- in `monix.types`, split the `Deferrable` type in `Suspendable` and `Memoizable`
- in `monix-execution` introduce `executeAsync` and `executeLocal` as macros
- use `executeLocal` and `LocalRunnable` in key points in the `Task` implementation to reduce forking
@alexandru

This comment has been minimized.

Copy link
Member Author

commented Aug 26, 2016

Deployed in 2.0-RC13

@alexandru alexandru closed this Aug 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.