Skip to content

to(Seq) doesn't force a stream, where to[Seq] did #236

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

Closed
martijnhoekstra opened this issue Jul 23, 2019 · 3 comments · Fixed by #238
Closed

to(Seq) doesn't force a stream, where to[Seq] did #236

martijnhoekstra opened this issue Jul 23, 2019 · 3 comments · Fixed by #238

Comments

@martijnhoekstra
Copy link

In 2.13, a Stream#to(Seq) returns identity.

In 2.12, Stream#to[Seq] returns a Vector, as does Stream#to(Seq) with compat.

I would argue that's a progression, not a regression, but it does introduce a semantic, potentially breaking change, both in the face of serialization and termination, when the stream isn't forced.

special-casing the scalafix for Stream#to(Seq) to Stream#toVector would make this change safe and obvious. That could either be obviously wrong or obviously right, both is a win.

Arguably, the same should be done for any CC#to(Seq) where Seq <: CC <: Stream.

@julienrf
Copy link
Contributor

julienrf commented Jul 23, 2019

I would argue that's a progression, not a regression

Both behaviors are legitimate. We could say that .to(Seq) always builds a “default” Seq, regardless of the type of the source collection. Or we could say that Stream is a Seq so that’s fine to optimize .to(Seq) to return the source collection in that case.

That being said, I have a preference for keeping the behavior we have in 2.13, because it makes .to(Seq) consistent with .toSeq (which already returned a Stream when applied to a Stream in 2.12). We could just document the fact that it is incompatible with the 2.12 behavior.

The fact that to(Seq) behaves differently on 2.13 and on 2.12 (with compat) is a problem, though. We should see if it’s possible to fix it.

@martijnhoekstra
Copy link
Author

No matter which is done, either to[Seq] and to(Seq) behave differently on 2.12, or to(Seq) behaves differently depending on whether you're on 2.12 or 2.13.

So if I understand you correctly, given the status 2.12 to[Seq] -> Vector and 2.13 to(Seq) -> (unforced) Stream, we want to have 2.12 to(Seq) -> (unforced) Stream?

And still possibly amend the scalafix?

@julienrf
Copy link
Contributor

we want to have 2.12 to(Seq) -> (unforced) Stream?

Yes.

And still possibly amend the scalafix?

To rewrite Stream#.to[Seq] to Stream#.to(Vector)? Yes, why not. We should also rewrite Stream#.to[LinearSeq] to Stream#.to(List).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants