To start off, I will say what I say on every question dealing with files: always use with
syntax.
By using this syntax you lessen the change for bugs; especially the bug that happens when you forget to call close()
on an open file pointer. Your code shows this bug. You open the temp.del
file, but never call close
on it. Because of this, when you program finishes running, you have an active file pointer that the OS needs to clean up.
To fix this, simply do this:
with open('temp.del', 'r') as file:
# Do stuff.
Once this block of code finishes, file
will automatically get closed.
To answer you question whether or not its necessary to write the temporary file, take a look a the urllib.urlopen
documentation. It says:
Except for the info(), getcode() and geturl() methods, these methods have the same interface as for file objects — see section File Objects in this manual. (It is not a built-in file object, however, so it can’t be used at those few places where a true built-in file object is required.)
This means that you should be able to iterate through this just like you would a file. Which, in your case, means you do not have to save the temporary file. Unfortunately, in Python 2.7, the return object from urllib.urlopen
is not a valid context manager (i.e doesn't implement __enter__
and __exit__
). So we have to use open
and close
:
file = urllib.urlopen("https://launchpad.net/elementary/+milestone/isis-beta1")
for line in file:
Now onto improvements:
In each of your if statements, you have the condition and i < 450
. You can bring this out into its own statement, and because you don't do anything after line 450, you can simply break if it evaluates to true:
for index, line in enumerate(file):
if index >= 450:
break
Getting the bug counts can be done easier using a regex:
import re
# Grabs all the numerical data, converts them to ints, then sums them together.
bugs += sum(map(int, re.findall('\d+', next_line)))
Each if statement holds the same code: you get the next line, split it, and then assign a value to a certain variable. After you finish the bugs, you add them all together. You don't differentiate after you parse them. You can combine these into a single if statement and simply keep a single running total like so:
if any(span in line for span in ['span class="statusINCOMPLETE">',
'span class="statusCONFIRMED">',
'span class="statusINPROGRESS">']):
bugs += sum(map(int, re.findall('\d+', next_line)))
If you need to differentiate the different kind of bugs, you can use a dict:
bug_types = {'span class="statusINCOMPLETE">': 0,
'span class="statusCONFIRMED">': 0,
'span class="statusINPROGRESS">': 0}
for key in bug_types:
if key in line:
bug_types[key] += sum(map(int, re.findall('\d+', next_line))))
When formatting string output, convention says to use the str.format()
function. The syntax look like this:
print '{} {}!'.format('Hello', 'World')
If you take all of my suggestions into account, here is your new code:
import urllib
import os
import re
bug_types = {'span class="statusINCOMPLETE">': 0,
'span class="statusCONFIRMED">': 0,
'span class="statusINPROGRESS">': 0}
# Even though we have to use `open` and `close` and can simply enclose the next
# for loop with the calls, I still would load the file into a temporary array.
# Then iterate over that.
file = urllib.urlopen("https://launchpad.net/elementary/+milestone/isis-beta1")
lines = [str(line) for line in file]
file.close()
# Because of the way `urlopen` streams the data, we cannot use the `next` command.
# Therefore, we need to use a flag to remember what kind of bug we found on the
# previous line.
bug_type = ''
for index, line in enumerate(lines):
if index >= 450:
break
if bug_type:
bug_types[bug_type] += sum(map(int, re.findall('\d+', line)))
bug_type = ''
else:
for key in bug_types:
if key in line:
bug_type = key
break
print('\n{} bugs left until Isis. Quit moaning.\n'.format(sum(bug_types.values())))