clj-commons / byte-streams

A Rosetta stone for JVM byte representations
416 stars 33 forks source link

Can't convert stream of byte arrays to seq of byte arrays #36

Open aengelberg opened 6 years ago

aengelberg commented 6 years ago

When I try to convert a stream of byte arrays to a seq of byte arrays, it just returns back the original stream without doing any conversion.

user=> (require '[manifold.stream :as ms])
user=> (require '[byte-streams :as bs])
user=> (def content (doto (ms/stream) (ms/put! (byte-array 5)) (ms/close!)))
#'user/content
user=> (bs/convert content (bs/seq-of bytes))  ; doesn't work
#<manifold.stream.default.Stream@942027b>
user=> (bs/convert content (bs/seq-of String)) ; also doesn't work
#<manifold.stream.default.Stream@942027b>
user=> (bs/convert content (bs/seq-of java.nio.ByteBuffer)) ; this works
(#<java.nio.HeapByteBuffer@5ff2cd23 java.nio.HeapByteBuffer[pos=0 lim=5 cap=5]>)
KingMob commented 3 years ago

First, given there's no conversion of the wrapped byte type, you should probably reach for Manifold's stream->seq fn instead of convert.

I think the docstring for convert could be a bit clearer on this, but it does mention that if the input is a stream, the stream contents must be specified.

What's missing in the docstring is the option key to accomplish this. FWIW, the option is :source-type, e.g., (convert content (seq-of bytes) {:source-type (stream-of bytes)}).

I also think returning the original input unchanged instead of throwing an IllegalArgumentException (like it says in the docstring) is a bug, but I need to make sure any fix doesn't break identity transformations. I'll have a PR soon.