luizirber / niffler

Simple and transparent support for compressed files.
Apache License 2.0
75 stars 7 forks source link

add Send bounds #35

Closed luizirber closed 4 years ago

luizirber commented 4 years ago

needletail had to rollback niffler adoption because PyO3 requires a Send bound. This PR adds Send bounds to all places where a Box<dyn Read> or Box<dyn Write> is used, and... it all passes the tests?

So, what are the drawbacks of adding Send? Other than requiring bumping niffler to 3.0.0? =P

(similar discussion: https://fasterthanli.me/articles/getting-in-and-out-of-trouble-with-rust-futures)

codecov[bot] commented 4 years ago

Codecov Report

Merging #35 into master will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #35   +/-   ##
=======================================
  Coverage   98.89%   98.89%           
=======================================
  Files           2        2           
  Lines         181      181           
=======================================
  Hits          179      179           
  Misses          2        2           
Impacted Files Coverage Δ
src/lib.rs 100.00% <ø> (ø)
src/compression.rs 97.93% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4b9ec1c...4d61c07. Read the comment docs.

luizirber commented 4 years ago

So, what are the drawbacks of adding Send? Other than requiring bumping niffler to 3.0.0? =P

I also tried using this branch in sourmash and decoct, and both worked fine (I had to fix one place in sourmash where I was asking for a R: io::Read instead of R: io::Read + Send, and hence why the bump to 3.0.0). Does anyone know about something that implements Read but is not Send?

Keats commented 4 years ago

Does anyone know about something that implements Read but is not Send?

I don't think there's any but don't quote me on that... I'm sure there's someone out there with a custom Read implementation that is not Send

natir commented 4 years ago

So, what are the drawbacks of adding Send?

I didn't see any, for me we can merge this pull request

Other than requiring bumping niffler to 3.0.0?

Yes but I want to integrate get_reader_seek and get_writer_seek and bgzip (we can do this easily) before release 3.0.