Writing shorter, readable, pythonic code

Question

I'm trying to produce shorter, more pythonic, readable python. And I have this working solution for Project Euler's problem 8 (find the greatest product of 5 sequential digits in a 1000 digit number).

Suggestions for writing a more pythonic version of this script?

numstring = ''
for line in open('8.txt'):
 numstring += line.rstrip()
nums = [int(x) for x in numstring]
best=0
for i in range(len(nums)-4):
 subset = nums[i:i+5]
 product=1
 for x in subset:
 product *= x
 if product>best:
 best=product
 bestsubset=subset
print best
print bestsubset

For example: there's gotta be a one-liner for the below snippet. I'm sure there's a past topic on here but I'm not sure how to describe what I'm doing below.

numstring = ''
for line in open('8.txt'):
 numstring += line.rstrip()

Any suggestions? thanks guys!

Answer 1

I'm working on a full answer, but for now here's the one liner

numstring = ''.join(x.rstrip() for x in open('8.txt'))

Edit: Here you go! One liner for the search. List comprehensions are wonderful.

from operator import mul
def prod(list):
 return reduce(mul, list)
numstring = ''.join(x.rstrip() for x in open('8.txt'))
nums = [int(x) for x in numstring]
print max(prod(nums[i:i+5]) for i in range(len(nums)-4))

• dyln: That's really slick. what do you think of using a lambda instead of mul? i.e. def prod(list): return reduce(lambda x,y: x*y, list)
• Rob Volgman: That works well too. I don't know why python didn't build it in - it's a pretty common requirement (even more so for Project Euler!), and it really helped having it as a built-in in R.
• steveha: Where Python offers a built-in like operator.mul, it is generally more efficient to use that rather than a lambda. For something like this, the efficiency doesn't really matter; your computer will find the answer in the blink of an eye, so you can use what you prefer. But in general, it's not bad to be in the habit of importing from operator when you are doing things with reduce() or map() or whatever.

Answer 2

from operator import mul
def product(nums):
 return reduce(mul, nums)
nums = [int(c) for c in open('8.txt').read() if c.isdigit()]
result = max((product(nums[i:i+5]) for i in range(len(nums))))

• Steven Rumbalski: @thebjorn: I intentionally didn't subtract 4 because it didn't impact the result. If I was going to subtract I would have probably done something like range(len(nums) - 5 + 1) and maybe even named the magic number at that point.
• steveha: There are some rather elegant tricks being used here. But your use of max() with a key means that your result is set to the sequence of 5 numbers, not to their product. It would be better to simply use result = max(product(nums[i:i+5]) for i in range(len(nums)))
• steveha: I have to say I really like the list comprehension that creates nums. Never mind using .replace() to get rid of line endings; just pull only the digits and convert them to integers in one go. Elegant.
• Steven Rumbalski: @steveha. I misread the problem. I thought it needed the actual sequence. I'll edit the answer.
• Steven Rumbalski: @steveha. Ah. I see. The OP keeps the actual sequence, but the Project Euler problem does not require it.

Answer 3

Here is my solution. I tried to write the most "Pythonic" code that I know how to write.

with open('8.txt') as f:
 numstring = f.read().replace('\n', '')
nums = [int(x) for x in numstring]
def sub_lists(lst, length):
 for i in range(len(lst) - (length - 1)):
 yield lst[i:i+length]
def prod(lst):
 p = 1
 for x in lst:
 p *= x
 return p
best = max(prod(lst) for lst in sub_lists(nums, 5))
print(best)

Arguably, this is one of the ideal cases to use reduce so maybe prod() should be:

# from functools import reduce # uncomment this line for Python 3.x
from operator import mul
def prod(lst):
 return reduce(mul, lst, 1)

I don't like to try to write one-liners where there is a reason to have more than one line. I really like the with statement, and it's my habit to use that for all I/O. For this small problem, you could just do the one-liner, and if you are using PyPy or something the file will get closed when your small program finishes executing and exits. But I like the two-liner using with so I wrote that.

I love the one-liner by @Steven Rumbalski:

nums = [int(c) for c in open('8.txt').read() if c.isdigit()]

Here's how I would probably write that:

with open("8.txt") as f:
 nums = [int(ch) for ch in f.read() if ch.isdigit()]

Again, for this kind of short program, your file will be closed when the program exits so you don't really need to worry about making sure the file gets closed; but I like to make a habit of using with.

• dyln: Yeah I think that the definition of sub_lists(lst, length) makes a lot of sense. It was confusing to use the magic number as in len(nums)-4.
• Rob Volgman: Using a definition of prod like that is significantly slower than using the builtin mul from operator.

Answer 4

As far as explaining what that last bit was, first you create an empty string called numstring:

numstring = ''

Then you loop over every line of text (or line of strings) in the txt file 8.txt:

for line in open('8.txt'):

And so for every line you find, you want to add the result of line.rstrip() to it. rstrip 'strips' the whitespace (newlines,spaces etc) from the string:

 numstring += line.rstrip()

Say you had a file, 8.txt that contains the text: LineOne \nLyneDeux\t\nLionTree you'd get a result that looked something like this in the end:

>>>'LineOne' #loop first time
>>>'LineOneLyneDeux' # second time around the bush
>>>'LineOneLyneDeuxLionTree' #final answer, reggie

• dyln: Thanks for the thoughtful explanation @TankorSmash. I should have been clearer in my question, what I meant was: I dont know how to describe what I'm doing here succinctly enough to search for past topics on it.

Answer 5

Here's a full solution! First read out the number:

with open("8.txt") as infile:
 number = infile.replace("\n", "")

Then create a list of lists with 5 consecutive numbers:

cons_numbers = [list(map(int, number[i:i+5])) for i in range(len(number) - 4)]

Then find the largest and print it:

print(max(reduce(operator.mul, nums) for nums in cons_numbers))

If you're using Python 3.x you need to replace reduce with functools.reduce.

• JBernardo: you can just replace '\n' with ''
• orlp: @JBernardo: sure, but that'll split on any whitespace, and "\n" makes the intent more clear.
• orlp: @JBernardo: ah now I see, yeah that's probably better.
• Steven Rumbalski: @nightcracker: range(len(number) - 5) is a bug. Test it on '123456789'. It misses the digit 9.
• thebjorn: map, reduce, and lambda aren't consider Pythonic by Guido ( artima.com/weblogs/viewpost.jsp?thread=98196 ).