2
\$\begingroup\$

I am very new to python (probably too new to consider efficiency etc.). But I am bit confused about which method to use while reading a file for post-processing. Here I am reading it line-by-line. There is other methods also for reading the file(e.g infile.read()), one of them is to load the complete file in memory. Can someone kindly discuss this?

My code in current condition is:

#!/usr/bin/python
import sys,math,re
try:
  inf=sys.argv[1]
except:
    print "usage:", sys.argv[0], "argument"; sys.exit(1)

infile=sys.argv[1]; oufile=sys.argv[2]
ifile=open(infile, 'r'); ofile=open(oufile, 'w')
pattern=r'species,subl,cmp=\s{4}(.*)\s{4}(.*)\s{4}(.*)\s{3}s1,torque=(.{12})(.{12})'

ssc1=[];ssc2=[];ssc3=[]; s1=[]; t=[]
for line in ifile:
  match = re.search(pattern, line)
  if match:
    ssc1.   append(int(match.group(1)))
    s1.     append(float(match.group(4)))
    t.      append(float(match.group(5)))

for i in range(len(t)):
  print('%g %12.5e %12.5e' % (ssc1[i], s1[i], t[i]))

ifile.close(); ofile.close()
\$\endgroup\$
  • \$\begingroup\$ Why would you not use proper coding style and conventions. Code looks too ugly. Also,while handling the IO operations with files, do have check on file size when you loading the complete file in memory. \$\endgroup\$ – Vinit Kumar Nov 17 '13 at 20:00
3
\$\begingroup\$

With regards to reading the file, I would say that reading it line by line is a great way to go. That way only part of it will be in memory at a time and you can thus handle a much larger file. If you know file sizes are small, and performance matters, and profiling shows another approach to be better, then you can consider an alternative. In the meantime, for line in somefile is quite idiomatic, and isn't likely to be the bottleneck.

That said, you discard some of the memory benefit when you store all the values in several lists, only to iterate over them, printing them one at a time. With no further context, I would tend to suggest you rewrite your loop like this:

for line in ifile:
    match = re.search(pattern, line)
    if match:
        ssc1 = int(match.group(1))
        s1 = float(match.group(4))
        t = float(match.group(5))
        print("%g %12.5e %12.5e" % (ssc1, s1, t))

But obviously if in your real context you need to load all the values before you can process them, or if you care about certain differences in behavior here, like whether one invalid matched group causing int or float to raise an exception should prevent all output, then you should keep your current approach.

I would also suggest some small changes:

  • Remove unused variables (inf, ssc2, ssc3, ofile, oufile).
  • Use more descriptive variable names.
  • Avoid bare except. At the very least, catch BaseException; alternately consider the LBYL of checking the number of values in sys.argv.
  • Avoid doing multiple things on a single line (replace ; with newlines).
  • Consider compiling the regular expression, and possibly using named groups instead of group indices; the latter makes it easier to update the regex's captures.
  • Consider refactoring most of this into a function whose name describes what it's doing, and handle passing relevant arguments from sys.argv to it in an if __name__ == '__main__' block. This would allow it to be usable as a module in addition to as a script.
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I'd prefer using str.format over the % style string formatting. \$\endgroup\$ – Bakuriu Nov 17 '13 at 22:16
  • \$\begingroup\$ thanks for your detailed explanation. But as I am new, and yet to learn every thing, can you kindly refer me the some source on compiling regex and named group (i.e. 5th point)? \$\endgroup\$ – BaRud Nov 17 '13 at 23:11
  • \$\begingroup\$ See the docs for re rx = re.compile(pattern); match = rx.search(line); as for named groups, see the same page for (?P<name>...) \$\endgroup\$ – Michael Urman Nov 18 '13 at 13:19
3
\$\begingroup\$

First, Where are you writing in ofile?

Second, ;) you can use with statement to opening and closing files like below:

pattern=r'species,subl,cmp=\s{4}(.*)\s{4}(.*)\s{4}(.*)\s{3}s1,torque=(.{12})(.{12})'
infile=sys.argv[1]; outfile=sys.argv[2]
with open(infile, "r") as ifile, open(outfile, "w") as ofile:
    for line in ifile:
        match = re.match(pattern, line)
        if match:
            ofile.write('%g %12.5e %12.5e' % (int(match.group(1)), float(match.group(4)), float(match.group(5))))
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I don't like encouraging such wide source lines (ofile.write), and this certainly makes some assumptions about the intent of the code, but agreed with both intended points. \$\endgroup\$ – Michael Urman Nov 17 '13 at 20:43
  • \$\begingroup\$ as I have said, I am very new to python. So, but is there any particular benefit of with statement to open file? I have not gone far enough to find this opening method in my book (python in a nutshell) \$\endgroup\$ – BaRud Nov 17 '13 at 21:51
  • 1
    \$\begingroup\$ @BaRud The with statement guarantees the closing of the file when execution leaves the with block, no matter how. \$\endgroup\$ – Janne Karila Nov 18 '13 at 7:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.