4 ms·

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("or

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.

kiidev 5y ago
Isn't that going to be even slower, since a 156mb file would probably make gc_lines a very big tuple before summing it?
- henbruas 5y ago
  gc_lines isn't a tuple here, it's a generator expression. It will be lazily evaluated.
- makapuf 5y ago
  You could do gc=sum(1 for _ on lines if ...) or use Counter
deleted 5y ago
[deleted]
ellimilial 5y ago
Isn’t this counting the lines with any G/C in them vs the total number of G/C literals?
_dain_ 5y ago
>total = len(list(gc_lines)) That won't work properly; you've already exhausted the gc_lines generator in the previous line.
- santiagobasulto 5y ago
  True, you'd need to re create the generator expression. Still, the other implementation seemed too naive.
  - _dain_ 5y ago
    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.
    - jhgb 5y ago
      > 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.
baggiponte 5y ago
The (…) return an iterator, right?
ad404b8a372f2b9 5y ago
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.
- deleted 5y ago
  [deleted]
pddpro 5y ago
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.

kiidev 5y ago

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

henbruas 5y ago
gc_lines isn't a tuple here, it's a generator expression. It will be lazily evaluated.
makapuf 5y ago
You could do gc=sum(1 for _ on lines if ...) or use Counter

deleted 5y ago

[deleted]

ellimilial 5y ago

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

_dain_ 5y ago

>total = len(list(gc_lines)) That won't work properly; you've already exhausted the gc_lines generator in the previous line.

santiagobasulto 5y ago
True, you'd need to re create the generator expression. Still, the other implementation seemed too naive.
- _dain_ 5y ago
  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.
  - jhgb 5y ago
    > 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.

baggiponte 5y ago

The (…) return an iterator, right?

ad404b8a372f2b9 5y ago

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.

deleted 5y ago
[deleted]

pddpro 5y ago

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.