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
Closed

Upgrade Cats dependency to 0.7 #212

alexandru opened this issue Aug 22, 2016 · 5 comments

Comments

@alexandru
Copy link
Member

Cats 0.7 is out and dependency should be upgraded.

@lukestephenson
Copy link
Contributor

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
Copy link
Contributor

lukestephenson 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
Copy link
Member Author

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
Copy link
Member Author

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 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
Copy link
Member Author

Deployed in 2.0-RC13

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

No branches or pull requests

2 participants