Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Feb 01 10:11
    @SystemFw banned @Hudsone_gitlab
  • Jan 31 2019 04:19
    404- forked
    404-/fs2
  • Jan 31 2019 03:01
    SethTisue commented #1232
  • Jan 30 2019 17:22
  • Jan 30 2019 13:45
  • Jan 30 2019 10:48
    pchlupacek commented #1406
  • Jan 30 2019 10:47
    pchlupacek commented #1406
  • Jan 30 2019 10:39
    pchlupacek commented #1407
  • Jan 30 2019 09:58
    lJoublanc commented #870
  • Jan 30 2019 09:42
    vladimir-popov commented #1407
  • Jan 30 2019 08:10
    vladimir-popov closed #1407
  • Jan 30 2019 08:10
    vladimir-popov commented #1407
  • Jan 29 2019 19:20
    SystemFw commented #1407
  • Jan 29 2019 19:20
    SystemFw commented #1407
  • Jan 29 2019 18:57
    SystemFw commented #1406
  • Jan 29 2019 17:47
    pchlupacek commented #1406
  • Jan 29 2019 17:42
    pchlupacek commented #1406
  • Jan 29 2019 17:39
    pchlupacek commented #1407
  • Jan 29 2019 17:39
    vladimir-popov edited #1407
  • Jan 29 2019 17:38
    vladimir-popov commented #1406
Brian P. Holt
@bpholt
@calvinlfer I’m not an expert on Pull, but assuming its correctness, the gist is useful and easy to follow!
Calvin Lee Fernandes
@calvinlfer
awesome! thank you for reviewing :D
Gavin Bisesi
@Daenyth
Calvin Lee Fernandes
@calvinlfer
@Daenyth this is very useful :O
needs to be part of the docs :P
Brian P. Holt
@bpholt
Is there likely to be another 0.10.x release? There’s a piece of my PR functional-streams-for-scala/fs2#1220 that I think might be safely backported (adding Stream.fromEither but not constraining the MonadError instance), but I won’t bother if there’s no plan for another release.
Fabio Labella
@SystemFw
mm, I have no idea
Michael Pilquist
@mpilquist
there will be at least one more
Brian P. Holt
@bpholt
:thumbsup:
Calvin Lee Fernandes
@calvinlfer
i think fs2 hit the 100 contributors mark :O
Brian P. Holt
@bpholt
@mpilquist I updated both my PRs based on your comments. Thanks for taking a look. I was curious why you suggested adding final to the partially applied class—what benefit does that bring?
Michael Pilquist
@mpilquist
Makes reasoning about binary compatiblity simpler -- I obsessively add final/sealed to avoid a case where someone extends a class in a way that wasn't intended and then a future change causes a binary compat problem
Admitedly in this case, it is super unlikely to matter :)
I added one more very minor comment
Brian P. Holt
@bpholt
Makes sense though
Michael Pilquist
@mpilquist
Are you interested in the same PR for Pull btw?
Brian P. Holt
@bpholt
I hadn’t given it any thought
It’s the same basic deal?
Michael Pilquist
@mpilquist
Yep
Brian P. Holt
@bpholt
Sure, let me take a look
Michael Pilquist
@mpilquist
:+1:
Brian P. Holt
@bpholt
Stream had MonadError, and now has Monad as a fallback if AE[F] isn’t available. Pull has Sync, which I can similarly constrain, but it’s not clear to me what the less powerful fallback should be (if any)?
Michael Pilquist
@mpilquist
Monad again :)
Sync <: Bracket <: MonadError <: { Monad, ApplicativeError}
Brian P. Holt
@bpholt
Oh, ok. I guess I was thinking Sync was required for other reasons and would just still be required.
Brian P. Holt
@bpholt

So constraining Pull.raiseError with AE ends up propagating out to compress.inflate (because it can raise an error if insufficient data is available). That makes this test, which explicitly operates on a PureStream, not work

There there are a couple other places that explicitly deal in Pipe[Pure, I, O] that I’m not sure what to do with:
https://github.com/functional-streams-for-scala/fs2/blob/f43fd1ce2bd173c545b6cd4ce482db0eba63d830/core/jvm/src/test/scala/fs2/MemorySanityChecks.scala#L138

https://github.com/functional-streams-for-scala/fs2/blob/f43fd1ce2bd173c545b6cd4ce482db0eba63d830/core/jvm/src/test/scala/fs2/PipeSpec.scala#L645

Any suggestions for what to do with these tests?

Michael Pilquist
@mpilquist
argh
Brian P. Holt
@bpholt
Stepper might be a problem too, because it can fail
Michael Pilquist
@mpilquist
yeah
there's always the Pull.done.map(_ => throw t) trick
but it seems like maybe this is a clue we shouldn't add this AE constraint
Brian P. Holt
@bpholt
I’m not sure I can make any useful observations on that… definitely following your lead here. 🤷🏻‍♂️
Michael Pilquist
@mpilquist
let's see what @SystemFw thinks
Brian P. Holt
@bpholt
There was a spot in Stream where we went straight to the algebra instead of going through raiseError… would something like that help here?
Michael Pilquist
@mpilquist
yeah, though the fact that this issue is showing up in "user code" like compress is what bothers me
in Stream, it only showed up in an implementation of a pretty fundamental combinator
Brian P. Holt
@bpholt
that’s true
although… Idk, inflate isn’t really a total function, is it? Not every input is valid, so is it really ok to run it in a pure context?
Michael Pilquist
@mpilquist
yeah, that's fair
alright i'm convinced by that :)
Stepper implementation is internal so that can use the algebra trick again (Pull.fromFreeC(Algebra.raiseError(t)))
Brian P. Holt
@bpholt
Actually I don’t see anywhere in Stepper that calls raiseError, but if Stepper.step returns Stepper.Fail, some of the test code does
e.g. StepperSanityTest
basically the same thing in both of those last two links I shared above
Michael Pilquist
@mpilquist
yeah
only solutions i have for that are (1) delete stepper entirely (2) use the Pure.done.map(_ => throw t) trick
Brian P. Holt
@bpholt
I’ll go with (2) for now and see how it shakes out
Brian P. Holt
@bpholt
FYI @mpilquist I just pushed #1224
Martijn Hoekstra
@martijnhoekstra
I'm still trying to understand the take example of Pull the uncons method gives a Pull[F,Nothing,Option[(Segment[O,Unit],Stream[F,O])]]. From what I understand is that should be seen as something from which you can pull elements of type Nothing, and once you are done pulling all elements, it will return an Option[(Segment[O,Unit],Stream[F,O])]
is that right?