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

Add Iterant.interleave #524

Merged
merged 6 commits into from Jan 18, 2018

Conversation

Projects
None yet
2 participants
@greenhat
Contributor

greenhat commented Jan 14, 2018

Closes #498.

@codecov

This comment has been minimized.

codecov bot commented Jan 14, 2018

Codecov Report

Merging #524 into master will decrease coverage by 5.97%.
The diff coverage is 93.47%.

@@            Coverage Diff             @@
##           master     #524      +/-   ##
==========================================
- Coverage   90.19%   84.21%   -5.98%     
==========================================
  Files         362      365       +3     
  Lines        9451    10449     +998     
  Branches     1829     1887      +58     
==========================================
+ Hits         8524     8800     +276     
- Misses        927     1649     +722
private[tail] object IterantInterleave {
def apply[F[_], A, B >: A](lh: Iterant[F, A], rh: Iterant[F, B]) (implicit F: Sync[F]): Iterant[F, B] = {

This comment has been minimized.

@alexandru

alexandru Jan 15, 2018

Member

Just a small comment — you don't need the subtyping relationship to avoid covariance issues here, i.e. you don't need a B type parameter.

This comment has been minimized.

@alexandru

alexandru Jan 15, 2018

Member

Or in other words, it's OK to have it on the method in Iterant, but not on this function here.

This comment has been minimized.

@greenhat

greenhat Jan 16, 2018

Contributor

@alexandru If I remove B in apply:

def apply[F[_], A](lh: Iterant[F, A], rh: Iterant[F, A]) (implicit F: Sync[F]): Iterant[F, A]

I get error:

[error] monix/monix-tail/shared/src/main/scala/monix/tail/Iterant.scala:1528:23: type mismatch;
[error]  found   : monix.tail.Iterant[F,A]
[error]  required: monix.tail.Iterant[F,B]
[error] Note: A <: B, but class Iterant is invariant in type A.
[error] You may wish to define A as +A instead. (SLS 4.5)
[error]     IterantInterleave(self, rhs)(F)
[error]                       ^

Do you want to remove B from Iterant.interleave?

This comment has been minimized.

@alexandru

alexandru Jan 16, 2018

Member

Ah, true, Iterant is invariant for now as a design decision having to do with covariance not mixing well with higher kinds, but you can use self.upcast[B] in that call.

This comment has been minimized.

@greenhat

greenhat Jan 16, 2018

Contributor

@alexandru Thanks! Done. I saw upcast but did not think of using it here.

greenhat added some commits Jan 16, 2018

Draft `Iterant.interleave` implementation;
Add equivalence test with `naiveImp`;
Switch to scalacheck test for `naiveImp`;
@greenhat

This comment has been minimized.

Contributor

greenhat commented Jan 17, 2018

@alexandru I'm done.

@greenhat greenhat changed the title from WIP: Add Iterant.interleave to Add Iterant.interleave Jan 17, 2018

@alexandru

This comment has been minimized.

Member

alexandru commented Jan 17, 2018

Thanks @greenhat, will review this soon — it has many branches and I have to pay attention 😀 tonight or tomorrow.

@alexandru alexandru merged commit 88875c1 into monix:master Jan 18, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@alexandru

This comment has been minimized.

Member

alexandru commented Jan 18, 2018

Looking good @greenhat, merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment