Skip to content
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

Use BufReader for PyFileLikeObject with 1MB buffer #69

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

manzt
Copy link
Contributor

@manzt manzt commented Nov 30, 2024

Wraps PyFileLikeObject in BufReader with a 1MB buffer for BAM, BCF,
VCF, and BigWig readers. Avoids changing the behavior of new to
prevent baking in assumptions about buffering needs or sizes, preserving
flexibility for unbuffered use cases.


Follow-up to #68

We want to avoid setting up the buffer inside new because it bakes in an opinion about buffering, which might conflict with cases where readers are already buffered, and it removes flexibility for users to configure buffer sizes or avoid buffering entirely if our choices are undesirable for their use case.

Wraps `PyFileLikeObject` in `BufReader` with a 1MB buffer for BAM, BCF,
VCF, and BigWig readers. Avoids changing the behavior of `new` to
prevent baking in assumptions about buffering needs or sizes, preserving
flexibility for unbuffered use cases.
Updates `FastqReader`, `GffReader`, and `GtfReader` to accept types
implementing `BufRead` directly, removing redundant wrapping with
`BufReader`. This aligns with readers that already assume buffered
input. Adjusted Python bindings to wrap `PyFileLikeObject` with
`BufReader` where necessary.
@pkerpedjiev
Copy link
Collaborator

Works for me 👍

You have some unused imports. Doesn't look like they're the result of this PR but maybe they just appeared because I was recompiling these files?

warning: unused import: `Read`
 --> /Users/pete/projects/oxbow/oxbow/src/fastq.rs:6:36
  |
6 |     io::{self, BufRead, BufReader, Read},
  |                                    ^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused imports: `Read` and `Seek`
 --> /Users/pete/projects/oxbow/oxbow/src/gff.rs:2:35
  |
2 | use std::io::{BufRead, BufReader, Read, Seek};
  |                                   ^^^^  ^^^^

warning: unused import: `Seek`
 --> /Users/pete/projects/oxbow/oxbow/src/gtf.rs:2:35
  |
2 | use std::io::{BufRead, BufReader, Seek};
  |

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

Successfully merging this pull request may close these issues.

2 participants