Deprecate Future instances

[BalmungSan]

If cats should have not provided instances for Future is a never-ending discussion.
However, I think the key points of such discussion are:

1. cats can't guarantee a predictable behavior of the instances.
2. cats should not remove the instances right away since that would break a lot of user code.

Thus, I propose (well, actually it was Gavin's idea :p) to deprecate them so that users are aware that their usage is not recommended.
And then move them to alleycats on the release of cats 3

This PR adds the deprecation and allows users to be prepared for the change.
Also, the deprecation message includes detailed instructions on how to suppress it and how to prepare for the change.

Let me know what you all think about it.

Relates to #4176 #2334

[rossabaker]

I reread both conversations, and am unconvinced we should churn code because some people have assumed behavior not backed by the laws. We're trying to use deprecation as a linter, when perhaps we should just provide a linter.

[armanbilge]

I'm not sure how well a (necessarily external) linter will reach users. My understanding of the discussions is that the goal is less about linting old/existing uses vs discouraging new ones and generally improving communication about the known traps. FWIW I don't think its unreasonable if the instances should stay in Cats permanently, just retain the deprecation.

I've never used the technique described here, but if -Wconf:cat=deprecation&origin=cats\\..*\\..*ForFuture:s works as it should this doesn't seem unreasonable. I also like the idea of using an explicit import to avoid the deprecation warning. These seem similar to the use of feature flags/imports in Scala for various things.

The Scala.js project cares a lot about compatibility and they recently implemented a similar technique to discourage use of the default ExecutionContext.global implicit in scala-js/scala-js#4556.

[BalmungSan]

because some people have assumed behavior not backed by the laws.

I think it is fair to assume "some people" are actually "most, if not all, Future users that don't use IO".
If that number of people is big or not we don't know, but it is the majority of people using these instances.

Also, as I said on discord, what is the point of having the instances if we can't guarantee their behavior.
Note that I am not even talking about if the behavior is lawful or not, just making it consistent; the change of traverse in 2.7.0 is an example of what I mean, and we all know it is not possible to guarantee a consistent behavior given how Future works.

IMHO, it is not a good UX for cats users to constantly tell them "ah yeah, we broke your code but, you know, it's your fault because you are using Future and we don't guarantee the execution order of Future". Because, I personally would immediately answer "then why it was supported in the first place?"

As @armanbilge said, the idea of the deprecation is mostly to be clear and loud that using Future + cats is not a good idea and that if you use them you are on your own.
Mainly for new users, but also for existing users that may not be aware of the problem yet.

Finally, I do think it is a good idea to eventually move the instances to alleycats, or, at least, remove them from the default implicit scope (as suggested by Gavin) so an explicit import is necessary.

---

but if -Wconf:cat=deprecation&origin=cats\..*\..*ForFuture:s works as it should this doesn't seem unreasonable.

I tested all the recommendations on the depreciation message using a small local project 2.13.8 using a local publish of this PR.

---

PS: I am all ears for suggestions to improve the deprecation warning.
Like, maybe we should explicitly mention IO?

[rossabaker]

MHO, it is not a good UX for cats users to constantly tell them "ah yeah, we broke your code but, you know, it's your fault because you are using Future and we don't guarantee the execution order of Future". Because, I personally would immediately answer "then why it was supported in the first place?"

Execution order was never supported in the first place. Should we also never change the iteration order of an unordered collection, even if we could optimize the documented use cases? Cats has done heroic work over the years maintaining binary compatibility and laws through years of evolution. Adding the constraint that we maintain arbitrary undocumented, unlegislated behaviors is a worrisome precedent.

I also like the idea of using an explicit import to avoid the deprecation warning. These seem similar to the use of feature flags/imports in Scala for various things.

As a trainer of junior Scala engineers and a recovering Haskell developer, feature imports make me grumpy, too.

I don't use Future much, so I'm not going to attempt a veto, but I still don't agree.

[BalmungSan]

Execution order was never supported in the first place.
Adding the constraint that we maintain arbitrary undocumented, unlegislated behaviors is a worrisome precedent.

Right, and I don't say we should. Actually, I am saying the opposite, that we simply can't guarantee any predictable behavior with Future
However, the problem resides in that, people using Future care about the execution order & if they are running concurrently or not; sure we all know Future is terrible at that and that even if you don't use cats a simple val vs def refactor can break the semantics of your code and you should be using IO instead, but that is outside of this discussion.

Also, I am not even saying that cats should try to provide the previous behavior. I am saying that we don't even know if the current one won't change in the future with another change; that is what I meant with predictable.

Thus, again, using Futures + cats is a bad idea if you care about execution order; just like iterating an unordered collection with a non-commutative operation is also a bad idea.
Thus, while I agree the right answer to the issue is "it is not cats fault" (that is basically the first thing I said in both discord discussions); I also think we could do better at communicating that sooner than later.

[johnynek]

I think a lot of this discussion turns on what a good UX is. IMO, the problem is there is no good choice. I have never really met a user that wants someone else to be paternal with them. They may exist, but I don't know them. Most take the view that functions like: e.g. NonEmptyList.fromListUnsafe exist and want the freedom to make that choice to use it in contexts where they have already proven the list is not empty.

So, here, the analogy is: Future is perfectly lawful if and only if you use it for pure concurrency, not for side-effects.

Given a user wants to use Future with side effects, there is really no "good UX" with cats: we could:

1. not have the instances (that was the case in the past and people complained with some frequency, but that was before cats-effect).
2. move the instances to alleycats. This one is baffling to me: how is forcing the user to add a new dependency and a new import somehow a better UX for them when doing something that is going to have the exact same behavior?
3. leave it as is and make sure the comments explain the dangers here (e.g. execution order is underspecified and you can't rely on it).

Maybe we should take a tougher stand against anything unsafe, but I would question alleycats at all at that point.

I personally would say: there is no solution to periodic complaints, they are going to happen no matter what, and I wouldn't make a change at this time (bias to stability in libraries). No matter what we do, there will be more complaints.

I'm 👎 on this one.

[BalmungSan]

So, here, the analogy is: Future is perfectly lawful if and only if you use it for pure concurrency, not for side-effects.

Given it seems you know many users, do you know anyone that uses Future like that? If so, are they the majority of those users?
Because I sincerely don't know anyone that uses Future for pure parallelism and a lot that use them for computational effects that are sensitive to their execution order and concurrent execution.

Edit

After re-reading my comment that the last paragraph sounded combative, not my intention. Rather it was a sincere question, I know I have a bias and I want to confirm it.
I sincerely don't know of people using Future for just plain parallelism; actually, I just recall Daniel thinks the same: typelevel/cats-effect#2115 (comment) (I hope you don't mind me linking you here :))

---

move the instances to alleycats. This one is baffling to me: how is forcing the user to add a new dependency and a new import somehow a better UX for them when doing something that is going to have the exact same behavior?

Intention, alleycats explicitly state that their instances do not hold the laws.
Nevertheless, leaving them inside cats with an error message is also a good idea IMHO.

Again, I am not against telling users "this is not cats fault, you are doing something it doesn't support", contrary I am proposing we explicitly tell them that is the case from the very beginning.
Plus, also allowing them to easily silence the warning; be it with a compiler setting or an explicit import (such import could be cats.instances.future._ instead of the alleycats one)

[armanbilge]

I don't think this discussion about side-effects at all. Because AFAICT the OP in #4176 is using Future lawfully (i.e., for pure concurrency, not for side-effects). This is also where the discussion settled in #2334 (comment).

Re-reading #2334 (comment), this comment strikes me:

I think losing the ability to traverse a List[Future[...]] would cost us one of the best "gateway drugs" for Cats.

Haven't the changes in #3960 exactly lost us that? For sure, you can still use traverse with Future, but it now runs sequentially instead of concurrently, which I think lacks some of the "kick" :)

[johnynek]

Given it seems you know many users, do you know anyone that uses Future like that? If so, are they the majority of those users?
Because I sincerely don't know anyone that uses Future for pure parallelism and a lot that use them for computational effects that are sensitive to their execution order and concurrent execution.

yes, see my original comment almost 4 years ago: #2334 (comment) -- I think it is domain specific. If you are doing a CRUD app, that would possibly never come up. If you are building data aggregation systems it might come up somewhat regularly. I've also used Future to do concurrent compilation in a programming language implementation (build a dag compilation units, then memoize a function of Unit => Future[Compiled] and call that for all imports, then sequence them).

So, I do know many different people who have used cats on a regular basis because we used cats at work at both of my two recent jobs and I know library authors that depend on cats, but everyone has their own experiences with seeing other people use cats, I guess.

I also know people who traverse Map. Really they should only do that with a commutative Applicative, but whatever man... they just do it! I'm sure some of them have bugs due to this. If someone has a bug we seem to somehow be willing to say: well, it's in alleycats, so that's on you. But we find it unacceptable in this case to say "we don't guarantee anything about execution order of the functions in traverse, so that's on you".

That's the part of the argument I find so strange: like, everything is fine if we move it to another package in this same repo because in that package we have warned you enough, but in this package, you can be forgiven for doing side-effects and expecting something).

I am fairly sure we would tell someone who wrote a side-effect in a function they pass to Functor.map and expected something about evaluation, that cats just doesn't promise what you are assuming.

[joroKr21]

What about trying to restore the original semantics of traversing a list of Futures? Is that possible?
If the issue is with stack safety - Future is stack-safe so perhaps it can be done?

[BalmungSan]

That's the part of the argument I find so strange: like, everything is fine if we move it to another package in this same repo because in that package we have warned you enough, but in this package, you can be forgiven for doing side-effects and expecting something).

To be honest, yes; that is my whole argument.
As I said, I don't want to start a discussion about if it is lawful or not, or useful or not, or whatever.
Actually, being fair, I think I have done a terrible job at explaining my motivations and proposal.

My point is just "cats can't guarantee consistent behavior, thus let's be clear about it".
And from that idea I propose:

1. That the best way to be clear about it right now is with a deprecation warning. Since it is already in the repo and people are already using them.
2. Also, users should have an easy way to silence the warning. I propose both a compiler configuration and an explicit import; either one is enough.
3. Additionally, I think it may make sense to eventually move the instances to alleycats since AFAIK that is the right place for what is not "safe" in cats. But, I am not strong about this one, so happy to remove it if most folks think it is not a good idea.

I am fairly sure we would tell someone who wrote a side-effect in a function they pass to Functor.map and expected something about evaluation, that cats just doesn't promise what you are assuming.

Well, I personally don't think that is a fair comparison.
Doing side-effects inside map is something most people agree is not a good idea; whereas depending on the execution order and concurrency of Future is the common usage for the majority of people that will use cats + Future.

Also, if the point is people will break the rules no matter what. Then, why alleycats exist? Why does cats-effect use the unsafe word in multiple places?
I think it is fair that we try to discourage users from doing something bad, especially if overcoming the barrier requires minimum effort; again I am not proposing deleting the instances entirely.

Even more, a common argument against the removal or movement of the instances is that cats have been doing an excellent job at being stable. But 2.7.0 just broke user code. Thus, shouldn't we fix it? of course not; we know it doesn't make sense for cats to try to preserve that behavior; which is why I insist cats should be loud and clear about that problem.

But, I think I am being repetitive here.
I will try not to keep repeating this in future replies.

What about trying to restore the original semantics of traversing a list of Futures? Is that possible?

Not sure if possible, but I don't think is a good idea to impose the burden of ensuring Future keeps behaving in its own little weird way every time a change is made.

[johnynek]

What about trying to restore the original semantics of traversing a list of Futures? Is that possible? If the issue is with stack safety - Future is stack-safe so perhaps it can be done?

We can't do that without ugly hacks. The change is in how traverse works in order to be type safe, and that change is in the Traverse instance. Unless we do some terrible hack, that code cannot depend on the Applicative[F] in question.

[johnynek]

Also, if the point is people will break the rules no matter what. Then, why alleycats exist? Why does cats-effect use the unsafe word in multiple places?
I think it is fair that we try to discourage users from doing something bad, especially if overcoming the barrier requires minimum effort; again I am not proposing deleting the instances entirely.

I think the right answer to that is the same as rust's unsafe. It is not for things that are actually wrong, it is for when the compiler can't see it is safe, but the programmer can, e.g.

// this function is totally safe *but the compiler can't see that*
def sortNonEmpty[A: Ordering](nel: NonEmptyList[A]): NonEmptyList[A] =
  NonEmptyList.fromListUnsafe(nel.toList.sorted)

So, I think the answer should be: we have unsafe things to signal to the user you need to prove some invariants yourself (either by checking at runtime, or maintaining them yourself with some other structure). So, I would push back on the mentality of "trying to discourage the user..." I feel like that direction is paternalism and it is hard to draw the line where that ends. For instance, I don't like people do write List(1, 2, 3) and instead 1 :: 2 :: 3 :: Nil. How much should I work to encourage that vs let people make that call themselves?

So, to me, that's the motivation of alleycats: where the law violations will not be observed because the caller has extra invariants in play that are not true in general.

So, we could move NonEmptyList.fromListUnsafe into alleycats? Or maybe because it has Unsafe in the name we allow it in the main project?

I feel like here is the same thing: we prefer cats effect, but you may have some use cases for future instances here, but keep in mind that when programming with side-effects you can easily have subtle bugs, and good luck.

[johnynek]

What about trying to restore the original semantics of traversing a list of Futures? Is that possible? If the issue is with stack safety - Future is stack-safe so perhaps it can be done?

Actually, thinking about this more there is a tension here. In traverse(fn) you could eagerly go through the list applying the function to each item. If you do that, you lose short-circuiting that you get with Monads like Option, Either, Try. So, users that enjoy that performance win still have to apply the function for the whole list, even if the first item "fails". If we did apply the function eagerly, then we could do the tree accumulation of the F[_] values after we have applied, the Future parallelism would be restored, but at the expense of another non-documented behavior: giving up short-circuits.

I actually had a PR for an idea that allows the Applicative to control this:

#3993

With that PR or something like it, the Applicative[Future] could eagerly apply the function potentially but Either could be lazy to skip work if you hit a Left.

That said, even that is under-specified. It seems like what the user really wanted was .parTraverse(fn) and a Parallel instance for Future since you could easily argue that parallel operation in Future by default is also wrong.

Basically, if you are using A => Future[B] as an effect type, you are going to have a bad time with cats.

I hear the people that say because of that we shouldn't have instances. I have laid out why I disagree with that view, but I doubt we will all agree.

[BalmungSan]

So, I think the answer should be: we have unsafe things to signal to the user you need to prove some invariants yourself (either by checking at runtime, or maintaining them yourself with some other structure).

Sure. But then, why is a warning that says this is generally unsafe but if you are good then here is the way to silence it, much different than that?

I don't like people do write List(1, 2, 3) and instead 1 :: 2 :: 3 :: Nil. How much should I work to encourage that vs let people make that call themselves?

With all due respect, your examples / comparisons are getting more and more out of line with each reply; you are not comparing something that can change the semantics of a program with code style.

Also, I don't buy the whole "trying to discourage the user" / "paternalism" argument.
On one side, I think you are fighting my wording more than my intention. As I have already said, I am not looking to force users not to use cats + Future; I am just looking for a way to make it clearer that cats does not guarantee execution order and concurrency.
On the other side, while I agree there is a good dose of subjectivity in this subject (in general), I don't think it is too hard to say when something is just a style preference and when something is a generally accepted recommendation; but, still, I do agree users should always be able to do what they want (and that is why I proposed not one but two ways to silence the warning).
And on a third side, even if it is not the point of this proposal, discouraging users from something is not always a bad thing; otherwise, there wouldn't be linters.

---

That said, even that is under-specified. It seems like what the user really wanted was .parTraverse(fn) and a Parallel instance for Future since you could easily argue that parallel operation in Future by default is also wrong.

I would agree with that, but AFAIK (I could be totally wrong on this, I am not an expert on the internal of cats) we can't have a Parallel[Future] mostly because map2 / mapN is already parallel for Future
Maybe is not that we can't but that it would be a bit misleading since Applicative[Future] is already parallel and not sure if parTraverse would then ensure parallel execution of the Futures

But, if that is possible I do think it would be a good idea to make that change.

---

Finally, I personally don't feel anybody has provided real feedback about my proposal (recognizing that I wasn't clear about it until my previous reply).
There has been discussion about related topics but not the proposal itself; well maybe the part about alleycats, but I already said I am ok with removing that part.

So I will try one last time.

Do folks believe it is a good idea that cats is clearer and louder about it not guaranteeing the execution order and concurrent execution of Future values?
If your answer to the previous question was yes, then do you believe a deprecation warning is a good idea?
If yes, then what do you think of the current message? Also, what do you think of the two proposed ways of slicing the warning? (remember that the import could be cats one instead of an alleycats one)
If not, why and what alternative do you suggest?

Second part of the proposal (assuming you agree with the first part; being loud and clear).
What do you think cats should do with its Future instances in the next major release.

1. Remove them? I don't like this idea but is on the table.
2. Just keep warning.
3. Move them to alleycats (and remove the warning?)
4. Keep them in cats, but take them out of the default implicit scope so users must always need to import them (and remove the warning?)
5. Do nothing at all; i.e. you don't even agree with part one of this proposal.

[johnynek]

I apologize if I have frustrated you, but despite you using the phrase "with all due respect" I feel like your replies to me have been pretty negative.

I may have also bummed you out but when these discussions turn negative a likely outcome is we are both less likely to contribute and we all lose.

Anyway, you have asked for feedback: my opinion is that we do nothing except tell people who use Future in an effectful way that it will likely cause bugs such as this one, and they should consider cats-effect (and maybe link to this talk: https://m.youtube.com/watch?v=qgfCmQ-2tW0 )

[armanbilge]

my opinion is that we do nothing except tell people who use Future in an effectful way that it will likely cause bugs such as this one

@johnynek question: in #2334 (comment) you said

I work often in data-engineering areas, where we use Future to get concurrency to do computations in parallel: aggregate a tree of operations, not stateful side effects in many cases.

This makes a lot of sense to me and AFAICT is the use-case for lawful Future. If you don't mind expanding on it, I'd like to learn more about how you do this effectively, so that changes like #3960 don't create bugs. I think if we can get this expertise written down somewhere, it'll save users like @jtjeferreira ugly surprises in the future :)

[johnynek]

my opinion is that we do nothing except tell people who use Future in an effectful way that it will likely cause bugs such as this one

@johnynek question: in #2334 (comment) you said

I work often in data-engineering areas, where we use Future to get concurrency to do computations in parallel: aggregate a tree of operations, not stateful side effects in many cases.

This makes a lot of sense to me and AFAICT is the use-case for lawful Future. If you don't mind expanding on it, I'd like to learn more about how you do this effectively, so that changes like #3960 don't create bugs. I think if we can get this expertise written down somewhere, it'll save users like @jtjeferreira ugly surprises in the future :)

I don't really have any guidance and I tried to think of why.

I think the answer is there are basically only two functions to watch out for: traverse and traverse_. Why? Basically you need a situation where you have A => Future[B] and the constraint is not Monad[Future]. So, you are worried about functions with arguments like A => F[B] with a Applicative[F] (or Apply, Semigroupal).

Monadic constraints aren't generally a problem (usually in the impure case, but pretty much never in the pure case). The only issue, I think, is when you have A => Future[B] but want that to be parallel. I can only think of traverse and traverse_ which fit that (although maybe there are others I've missed).

[armanbilge]

Basically you need a situation where you have A => Future[B] and the constraint is not Monad[Future]. So, you are worried about functions with arguments like A => F[B] with a Applicative[F] (or Apply, Semigroupal).

Thank you for taking time to think about that. I found this answer to be very insightful about what's happening under-the-hood.

So to close this out, going forward what exactly should we explain to users is the correct way to use Future instances? If I understand everything correctly so far, either of the following should be "safe":

1. Use Future for pure concurrency, not for side-effects. But (unlike anywhere else in Cats) don't use traverse, because your computations may not run in parallel.

   Question: was it common knowledge on your data engineering teams that traverse should be avoided when working with Future?

2. Use Future purely, but not for concurrency nor for side-effects. Your computations may run in parallel, but Cats makes no guarantees about this.

   Conceptually: only use Future in situations that you could essentially use e.g. Try / Eval / Id?

Then, if @BalmungSan is interested to continue we can make this into a docs PR.

Thanks again.

[johnynek]

Can anyone think of other examples beyond traverse and traverse_? It would be good to make sure somehow I'm not wrong about that.

I would simply say: any non-pure use of Future[A] with cats is error prone (particularly the semantics of traverse wrt to execution order are unspecified). We recommend using cats-effect as a replacement for every use case of Future.

[armanbilge]

We recommend using cats-effect as a replacement for every use case of Future

And just to be 100% clear (apologies, if you said it before and I missed it): you are 👎 on putting this in a deprecation message on these instances, even if we also promise that they will not be removed from Cats?

I know you said:

I wouldn't make a change at this time (bias to stability in libraries)

But it wasn't obvious to me if a deprecation is a form of instability. This goes to your point about how

I think a lot of this discussion turns on what a good UX is

[johnynek]

I don't think deprecation warnings are all that effective. If you see it, but want to keep using Future you have either junked someone's build, or got them to turn off some or all warnings. So basically only the people who say "oh, I can easily move to cats-effect" but somehow don't know that's a good idea. I think you are taxing one group to try to advertise to the other. I don't think it's great.

Sadly, in all the industrial code bases I have seen they are PACKED with deprecation warnings. I just don't think they are effective.

[armanbilge]

Ok, thanks. Ross also had reservations about using a deprecation here, so let's just focus on getting this messaging into the documentation.