Hacker News Reader

Comment by santiagobasulto | Hacker News Reader

I don't doubt Nim, looks like a great language. But that is just an awful Python implementation. I'd do it in this way:

    lines = (line for line in lines("orthocoronavirinae.fasta") if not line.startswith(">"))
    gc_lines = (1 if ('G' in line or 'C' in line) else 0 for line in lines)
    gc = sum(gc_lines)

    total = len(list(gc_lines))

    # Alternatively, a more "memory efficient" total would be:
    total = sum(1 for _ in lines)

Edit: my code is not perfect (I’m typing from my phone, I’m surprised I could even match parentheses).

My point is: this is a highly I/O bound program. The implementation matters. With the correct implementation there shouldn’t be much difference between the languages.

View on HN

>total = len(list(gc_lines))

That won't work properly; you've already exhausted the gc_lines generator in the previous line.

True, you'd need to re create the generator expression. Still, the other implementation seemed too naive.

There's probably some itertools trick to do it with only one iteration.

EDIT: you can do it with functools.reduce and a generator of tuples:

    from functools import reduce

    with open("orthocoronavirinae.fasta") as f:
        lines = (line.strip() for line in f if not line.startswith(">"))
        sums = ((len(line), sum(1 for ch in line if ch in "CG")) for line in lines)
        total, gc = reduce(lambda x, y: (x[0] + y[0], x[1] + y[1]), sums)

Besides, I really don't think that any of our solutions will be that much faster than the one in the OP. All of them are using lazy iteration on the file object, just written differently. The differences amount to micro-optimizations. The real way to speed it up would be to use something like pandas, where the loading and summing calls into fast C implementations.

> There's probably some itertools trick to do it with only one iteration.

Or an arithmetic trick. Since there's the `1 if ('G' in line or 'C' in line) else 0 for line in lines` construct in the code, if you simply replace 1 with (len(line)<<K)+1 and 0 with len(line)<<K (for a suitable value of K), you can extract the two results using div/mod.

That's not good python, it's unreadable and less optimized than the original code. Your "1 if ... else 0" condition serves no purpose and you iterate twice over the same loop for no reason. Use normal loops and save everyone who comes after a lot of reading time and bugs.

Given that there is a fixed set of Alphabets, I would rather use Counters. It avoids for loops (the biggest source of non-efficiency in python) and is imho is much more idiomatic.

Isn't that going to be even slower, since a 156mb file would probably make gc_lines a very big tuple before summing it?

You could do gc=sum(1 for _ on lines if ...) or use Counter

gc_lines isn't a tuple here, it's a generator expression. It will be lazily evaluated.

Isn’t this counting the lines with any G/C in them vs the total number of G/C literals?

The (…) return an iterator, right?

Reply on news.ycombinator.com