Cats: We need to talk about Stream

Created on 9 Aug 2019  路  7Comments  路  Source: typelevel/cats

Problem

The current Cats 2.0.0 release candidate (RC1) assumes that Scala 2.13 users will be able to alias the new LazyList to Stream (which is deprecated in 2.13.0). For example, the cats.instances.stream object doesn't actually provide instances for Stream on 2.13:

scala> import cats.Traverse, cats.instances.stream._
import cats.Traverse
import cats.instances.stream._

scala> Traverse[Stream]
               ^
       error: could not find implicit value for parameter instance: cats.Traverse[Stream]

But it does provide LazyList instances:

scala> Traverse[LazyList]
res1: cats.Traverse[LazyList] = cats.instances.LazyListInstances$$anon$1@43b7181d

There's also a lazyList object that provides exactly the same instances:

scala> import cats.Traverse, cats.instances.lazyList._
import cats.Traverse
import cats.instances.lazyList._

scala> Traverse[LazyList]
res0: cats.Traverse[LazyList] = cats.instances.LazyListInstances$$anon$1@75decfcd

So the stream object is still around just to facilitate cross-compilation, but only for users who can alias LazyList. For everyone else it's just confusing鈥攚hen you import the contents of something called instances.stream you're probably going to assume that you'll get some Stream instances.

We're not even really consistent about this, in that cats-kernel takes a slightly different approach: like cats-core, it provides a stream object in cats.kernel.instances that also contains LazyList instances but not Stream ones, but unlike cats-core it doesn't have a lazyList object.

None of this would be that big of a problem if everyone using 2.13 could just alias LazyList (there would still be the inconsistencies, but we have lots of inconsistencies), but that's not realistic, since there are lots of libraries that are published for 2.13 that still use Stream.

Solutions

There are a few things we could do to address these problems in Cats 2.0:

Provide stream instances

We could have cats.instances.stream and cats.kernel.instances.stream provide exactly the same instances across 2.11, 2.12, and 2.13, and introduce new cats.instances.lazyList and cats.kernel.instances.lazyList objects for 2.13. This seems to me like the right thing to do. The downside is that it means we have deprecation warnings, which means we either have to disable fatal warnings or move these instances to entirely separate modules that cats-kernel and cats-core depend on. I'd advocate for turning off fatal warnings, but I know that's not a battle I'm going to win. Ideally we'd be consistent about our names everywhere, not using "Stream" to mean "LazyList", but I know that's even less likely to happen.

Don't provide either instances or instance packages

If we decide not to provide instances for Stream on 2.13, it seems to me pretty aggressively user-unfriendly to provide some objects called instances.stream, because "a missing import error is easier to diagnose than a missing instance error". We could drop cats.instances.stream and replace cats.kernel.instances.stream on 2.13, and users who are able to alias LazyList and want to cross-compile can use the cats.instances.all import.

Nothing

We could just leave things the way they are, possibly introducing a lazyList object in cats-kernel to get rid of that inconsistency.

Most helpful comment

move these instances to entirely separate modules that cats-kernel and cats-core depend on

Won't that be circular dependency?
Principally I don't think cats should continue provide instance for deprecated things. If other libs choose to continue depends on that, a separate repo can provide such instances for their users (I myself am one of them).

The naming of stream related stuff in cats is for the sake of cross compiling. I am okay if we remove the instance.stream reference in 2.13, to me it's no big deal either way. The name stream will be a lie in many libs on 2.13, e.g. scalacheck.

I am :+1: adding a lazylist object to kernel for consistency.

All 7 comments

(I should note that I'm happy to do all of the work required to make either of these proposals happen.)

move these instances to entirely separate modules that cats-kernel and cats-core depend on

Won't that be circular dependency?
Principally I don't think cats should continue provide instance for deprecated things. If other libs choose to continue depends on that, a separate repo can provide such instances for their users (I myself am one of them).

The naming of stream related stuff in cats is for the sake of cross compiling. I am okay if we remove the instance.stream reference in 2.13, to me it's no big deal either way. The name stream will be a lie in many libs on 2.13, e.g. scalacheck.

I am :+1: adding a lazylist object to kernel for consistency.

Won't that be circular dependency?

Oh, right. Something like that came up briefly in the conversation yesterday but I didn't really think it through.

it means we have deprecation warnings, which means we either have to disable fatal warnings or move these instances to entirely separate modules

That's not the only option, perhaps. You could use the silencer library to annotate those specific objects and suppress the deprecation warnings.

You could use the silencer library to annotate those specific objects and suppress the deprecation warnings.

Well, those instance really should be deprecated.

Fixed in #2983.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

non picture non  路  3Comments

LukaJCB picture LukaJCB  路  3Comments

SimY4 picture SimY4  路  4Comments

diesalbla picture diesalbla  路  4Comments

alexandru picture alexandru  路  4Comments