v0.4 with rewritten parsers + python bindings #45

Keats · 2020-06-09T14:16:17Z

Closes DEV-3520, closes DEV-3519, closes DEV-3521.

linear · 2020-06-09T15:56:06Z

DEV-3520 Improve Rust API
DEV-3519 Improve needletail API
DEV-3521 Add Python bindings with Maturin (or a separate Python library)

Keats · 2020-06-23T12:40:54Z

cc @bovee in case you haven't seen that

bovee · 2020-06-27T00:34:54Z

This is really cool! My only nit is I think your "random TSV" example is actually from a SAM file header, but SAM files are technically TSVs so that's probably okay. :)

The new streaming iterator syntax is neat; I've been playing with building toy parsers with similar style so it's really nice to see needletail has that. Super excited for new stable maturin too; didn't know they were finally nailing that down.

luizirber · 2020-06-27T05:24:52Z

I just tried out this branch (see this commit), and was a bit sad to not see a parse_fastx_reader function, so I tried out a few things:

First, using io::Cursor and Box to avoid the Seek bound. Check this commit. This is a trick I learned with @natir, and we use it in niffler. And then...
This commit uses niffler to replace the compression detection logic, but implies adding a dependency. I checked the dep tree, and I think the only extra one is thiserror and enum_primitive. I also added parse_fastx_reader as the main method, and parse_fastx_file then becomes just a small shim that opens a File and passes to parse_fastx_reader (~~still need to fix one unwarp and understand how to pass the compression feature from needletail further down to the right niffler feature~~ fixed in this commit, which also brings back stdin support)

The second commit is pretty aggressive (sorry!) with all the code replacing, but I think the first one is less invasive. I can open a PR against this one if you want.

luizirber · 2020-06-27T16:44:19Z

oh, and I really liked the new iterator style! It makes working with errors a breeze, I used to have a lot of .expect in my parse_sequence_file function because it didn't allow returning errors...

Keats · 2020-06-29T11:28:48Z

I like the parse_fastx_reader a lot, I forgot to implement it again but it was on my TODO list in the back of my brain.
As for niffler I'm not sure, It's nice to be able to control the decoder. For example you have https://github.com/luizirber/niffler/blob/master/src/lib.rs#L163 which means multi gz file would fail to parse and if I want to change that I would to either fix it in niffle (where it might not be appropriate?) or remove it. We do need the MultiGzDecoder here since we do have files in that format (few but some).

boydgreenfield

Couple of minor typos. The body of this I didn't closely review, however, and would be better with somebody with more Rust experience's eyes...

@Keats One question, has our support for streaming from stdin changed in any measurable way with the port to the seqio parsing model? Just one thing we should be sure is stable that may not have the same test coverage.

boydgreenfield · 2020-06-29T12:54:08Z

README.md

@@ -8,61 +8,53 @@ Needletail is a MIT-licensed, minimal-copying FASTA/FASTQ parser and _k_-mer pro
 The goal is to write a fast *and* well-tested set of functions that more specialized bioinformatics programs can use.
 Needletail's goal is to be as fast as the [readfq](https://github.com/lh3/readfq) C library at parsing FASTX files and much (i.e. 25 times) faster than equivalent Python implementations at _k_-mer counting.

-# Example
+## Example


Is this intentional? Looks a bit like the whole block got commented out in an editor. (Referring not just to the header but to the rust code block below too)

The # part is intentional, the commented out code not so much...

boydgreenfield · 2020-06-29T12:55:08Z

README.md

+
+## Acknowledgements
+Starting from 0.4, the parsers algorithms is taken from [seq_io](https://github.com/markschl/seq_io). While it has been slightly modified, it is mainly
+coming that library. Links to the original files are available in `src/parser/fast{a,q}.rs`.


Typo: "coming from that library".

luizirber · 2020-06-29T16:42:51Z

As for niffler I'm not sure, It's nice to be able to control the decoder. For example you have https://github.com/luizirber/niffler/blob/master/src/lib.rs#L163 which means multi gz file would fail to parse and if I want to change that I would to either fix it in niffle (where it might not be appropriate?) or remove it. We do need the MultiGzDecoder here since we do have files in that format (few but some).

Fair point about controlling your deps. But I also tested changing that GzDecoder to a MultiGzDecoder and niffler worked fine (but need to test with more inputs), so that would be an option too.

Keats · 2020-06-29T16:53:58Z

I'm ok with niffler if it can be changed to MultiGzDecoder, I didn't know if that was ok to change or not as it is pretty niche.

luizirber · 2020-06-29T17:07:51Z

I'm ok with niffler if it can be changed to MultiGzDecoder, I didn't know if that was ok to change or not as it is pretty niche.

It's niche, but it works fine for regular gzip files too (I checked the flate2 code).
PR opened in niffler: luizirber/niffler#28

luizirber · 2020-06-30T15:41:34Z

niffler 2.1.1 released, with MultiGzDecoder as the default decoder for gz files.

Keats · 2020-06-30T15:45:26Z

Cheers, can you make a PR with your changes?

…#46) * Use cursor and Box to avoid Seek bound * use niffler * fix feature, bring back stdin support, remove unwrap * use niffler 2.1.1, with MultiGzDecoder as default * use niffler 2.2.0, and add stdin tests

Keats · 2020-07-02T09:19:28Z

@bovee do you have a new email I can put in the Cargo.toml?

bovee · 2020-07-03T02:08:08Z

First initial, last name at gmail. Maybe your email should be in the Cargo.toml instead though since you've fixed up most of the code? ;)

Keats · 2020-07-06T09:31:49Z

I've done both!

Keats · 2020-07-06T10:13:24Z

@luizirber looks like pyo3 0.11 is requiring Send in its latest version for pyclass and the Chain/ dyn std::io::Read type breaks it. I'm looking into it.

Keats added 5 commits May 29, 2020 14:20

Run cargo-diet

a706a69

Update rust-bio

7fa95b9

Move stray test

2f6c348

assert -> assert_eq

5f0bf6c

Shorter fastq fn

5ef0e7b

Keats force-pushed the vp_py3 branch from 1754a42 to 5c8c8d8 Compare June 9, 2020 14:30

New API for parsing + Python bindings

0a6ecc5

Keats force-pushed the vp_py3 branch from 5c8c8d8 to 0a6ecc5 Compare June 9, 2020 14:36

Keats added 2 commits June 25, 2020 17:55

Remove misleading text in error

0b44384

Pass seq position to seq record

f15fc2c

boydgreenfield reviewed Jun 29, 2020

View reviewed changes

This was referenced Jun 29, 2020

Replace GzDecoder with MultiGzDecoder? luizirber/niffler#27

Closed

Allow user to choose wich backend library is used luizirber/niffler#24

Closed

luizirber mentioned this pull request Jun 30, 2020

Drop Seek bound, and add back stdin support and parsing from a Reader #46

Merged

luizirber and others added 2 commits July 2, 2020 10:51

Drop Seek bound, and add back stdin support and parsing from a Reader (…

5ceae98

…#46) * Use cursor and Box to avoid Seek bound * use niffler * fix feature, bring back stdin support, remove unwrap * use niffler 2.1.1, with MultiGzDecoder as default * use niffler 2.2.0, and add stdin tests

Update changelog

3d49119

Update authors

7f8e042

Keats merged commit 814eb98 into master Jul 6, 2020

Keats deleted the vp_py3 branch July 6, 2020 09:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v0.4 with rewritten parsers + python bindings #45

v0.4 with rewritten parsers + python bindings #45

Keats commented Jun 9, 2020 •

edited by boydgreenfield

Loading

linear bot commented Jun 9, 2020

Keats commented Jun 23, 2020

bovee commented Jun 27, 2020

luizirber commented Jun 27, 2020 •

edited

Loading

luizirber commented Jun 27, 2020

Keats commented Jun 29, 2020

boydgreenfield left a comment

boydgreenfield Jun 29, 2020

Keats Jun 29, 2020

boydgreenfield Jun 29, 2020

luizirber commented Jun 29, 2020

Keats commented Jun 29, 2020

luizirber commented Jun 29, 2020

luizirber commented Jun 30, 2020

Keats commented Jun 30, 2020

Keats commented Jul 2, 2020

bovee commented Jul 3, 2020

Keats commented Jul 6, 2020

Keats commented Jul 6, 2020 •

edited

Loading

v0.4 with rewritten parsers + python bindings #45

v0.4 with rewritten parsers + python bindings #45

Conversation

Keats commented Jun 9, 2020 • edited by boydgreenfield Loading

linear bot commented Jun 9, 2020

Keats commented Jun 23, 2020

bovee commented Jun 27, 2020

luizirber commented Jun 27, 2020 • edited Loading

luizirber commented Jun 27, 2020

Keats commented Jun 29, 2020

boydgreenfield left a comment

Choose a reason for hiding this comment

boydgreenfield Jun 29, 2020

Choose a reason for hiding this comment

Keats Jun 29, 2020

Choose a reason for hiding this comment

boydgreenfield Jun 29, 2020

Choose a reason for hiding this comment

luizirber commented Jun 29, 2020

Keats commented Jun 29, 2020

luizirber commented Jun 29, 2020

luizirber commented Jun 30, 2020

Keats commented Jun 30, 2020

Keats commented Jul 2, 2020

bovee commented Jul 3, 2020

Keats commented Jul 6, 2020

Keats commented Jul 6, 2020 • edited Loading

Keats commented Jun 9, 2020 •

edited by boydgreenfield

Loading

luizirber commented Jun 27, 2020 •

edited

Loading

Keats commented Jul 6, 2020 •

edited

Loading