Rearrange page numbers - follow-up

This is a follow-up of: Rearrange page numbers for printing a book

Input:

• Total pages of a book: int
• Pages per booklet: int (divisible by 4 evenly)

Processing:

Rearrange the pages in each booklet to follow a specific format, [n, f, f+1, n-1], where n is the last page of the booklet, and f is the first.

Output:

Print a list/tuple of the rearranged integers

Bugs:

If pages is not a multiple of 4 the last booklet (list that get processed by arrange_booklet()) always has errors at the end.

Misc:

int pages will generally be a large number (50~500). I wrote the program because my printer software doesn't allow me to just print the pages in this order. I have to manually type every integer, comma-separated. Which would obviously get tedious very quickly.

from math import ceil

# f is the first page (smallest input integer), n is the last (largest)
def arrange_booklet(f, n):
'''Creates a list by using the page # order formula.'''
_ = list()
while n > f:
_.extend([n, f, f+1, n-1])
f += 2
n -= 2
return _

def make_book():
'''Combines all the lists into one and returns the result.'''
pages = list(range(1, int(input("Enter a number of pages: "))+1))
while True:
len_booklet = int(input(
"Enter booklet length, must be a multiple of four: "))
if len_booklet % 4 == 0:
break
book = list()  # permanent list, to be returned
_ = list()  # temporary list, to populate permanent list
for page in pages:
_.append(page)
if page % len_booklet == 0:
_ = arrange_booklet(_[0], _[-1])
book.extend(_)
_.clear()
# the last booklet, if pages % 4 != 0, is still in the temp list and broke
# out of the loop it has to be handled separately below, or it will never
# reach the book
if _:
_ = arrange_booklet(_[0], _[-1])
book.extend(_)
print(book)

if __name__ == '__main__':
make_book()

• Just realized i removed the ceil function in the program but forgot to remove the import statement.. Just ignore that please – qazwsx598 Apr 13 '18 at 3:11
• Running the code with 95 and 16 as input, I get the output (at the end) 95, 81, 82, 94, 93, 83, 84, 92, 91, 85, 86, 90, 89, 87, 88, 88]. Is this desired behaviour? 88 is repeated. – hjpotter92 Apr 13 '18 at 4:17
• Apparently you have some kind of affinity for _. Any reasons why you're using that instead of a reasonable named variable? ¯\_(ツ)_/¯ – Grajdeanu Alex Apr 13 '18 at 6:41
• Whislt the code looks better and is more documented, I am disappointed to see that my main point from previous comment was fully ignored. .. – SylvainD Apr 13 '18 at 7:04
• HJPotter, that is the bug, it is not intended behavior and I don't know how to fix it. яүυк The _ lists were just temporary so I didn't see a point in naming them, If I did I would've named them temp. @Josay Sorry I tried to take your advice fully into consideration. The only advice i explicitly left out was writing a test because I couldn't figure out how. I tried figuring out what you did but I have no experience with dictionaries/sets. I did test various numbers. – qazwsx598 Apr 13 '18 at 12:57

Functions

you should split a lot more of the small pieces of code to separate functions, For example getting and parsing the input has no place in the make_book

Generators

Instead of using temporary lists, using generators is a lot cleaner.

def my_func():
_ = []
for i in range(10):
_.append(i**2)
return _


with a generator this becomes:

def my_gen():
for i in range(10):
yield i**2


This is a lot cleaner, and easier to maintain.

Assembling the booklet

instead of iterating over the indices f and n, you can use a collections.dequeand use pop and popleft

from collections import deque
def assemble_booklet(pages):
"""assembles the booklet according to the order [-1, 0, 1, -2] and going in.

Assumes an iterable with the length a multiple of 4"""
pages = deque(pages)
assert not len(pages) % 4, 'booklet length, must be a multiple of 4'
while pages:
selection = pages.pop(), pages.popleft(), pages.popleft(), pages.pop()
yield from selection


again, the generator style allows us to express this a lot clearer than using the intermediary lists

grouping in pages

grouping the pages can be easily done with the grouper itertools recipe

from itertools import zip_longest

def grouper(iterable, n, fillvalue=None):
"""Collect data into fixed-length chunks or blocks

grouper('ABCDEFG', 3, 'x') --> ABC DEF Gxx"
https://docs.python.org/3/library/itertools.html#itertools-recipes
"""
args = [iter(iterable)] * n
return zip_longest(*args, fillvalue=fillvalue)


Then:

for page in pages:
_.append(page)
if page % len_booklet == 0:
_ = arrange_booklet(_[0], _[-1])
book.extend(_)
_.clear()


becomes:

def make_book(pages, booklet_length):
'''Combines all the lists into one and returns the result.'''
assert not booklet_length % 4, 'booklet length, must be a multiple of 4'
for group in grouper(pages, booklet_length, fillvalue='blank'):
yield tuple(assemble_booklet(group))
# yield from assemble_booklet(group) # dependent on the output you want


If you want the pages in 1 long list instead of separated per booklet, you can change the last line of make_book to yield from assemble_booklet(group)

And in effect, this is the whole code for this method, that gets an iterable of pages, and form booklets out of them

if __name__ == '__main__':
pages = range(1, 16)
booklet_length = 8
result = list(make_book(pages, booklet_length))
print(result)

[(8, 1, 2, 7, 6, 3, 4, 5), ('blank', 9, 10, 15, 14, 11, 12, 13)]

• What's happening in deque(pages). And does the yield statement below selection yield each page that is popped from the queue as it's popped? – qazwsx598 Apr 13 '18 at 16:16
• a deque is a kind of queue, a collection that allows easy adding and removing on either side. This could work with a list using pop(0) and pop(-1), but is more natural in a deque. Sincezip_longest yields tuples, pages needs to be put in another container anyway, so we might as well use the correct one. The yield from iterable is the same a for i in iterable; yield i – Maarten Fabré Apr 13 '18 at 21:04

@Josay This is the best that I can get the code to be. If I have still ignored your main point then I don't understand what your main point is, please elaborate.

MEMORY:

I feel like the arrange_booklet function uses much more memory than necessary.

BUGS:

I think I finally managed to figure out the bug where extraneous values appear at the end if pages wasn't a multiple of len_booklet

CHANGES:

• Added test() function that confirms if the book is the correct length
• Removed inputs from functions
• Moved some code from the old make_book() function and revised it, created an independent function named grouper()
• Combined what was rest of arrange_booklet() and make_book() into the new arranged_booklet()
• Inputs are behind an if __name__ == '__main__' guard

import itertools
import random

def grouper(pages, len_booklet, fillvalue=None):
'''pages is an iterable, len_booklet is an int
Creates "groups" of len_booklet length out of pages.'''
args = [iter(pages)] * len_booklet
result = list(itertools.zip_longest(*args, fillvalue=fillvalue))
for group in result:
yield list(group)

def arrange_booklet(grouped_book, len_booklet):
'''grouped book is an list, len_booklet is an in
Arranges grouped_book into the proper format.'''
for booklet in grouped_book:
while None in booklet:
booklet.remove(None)
n = booklet[-1]
f = booklet[0]
# beginning of special case
if len(booklet) <= len_booklet:
while len(booklet) > 4:
yield [n, f, f+1, n-1]
booklet = booklet[2:-2]
f += 2
n -= 2
if f == n:
yield [f]
elif f == n-1:
yield [n, f]
elif f == n-2:
yield [n, f, f+1]
else:
yield [n, f, f+1, n-1]
# end of special case
else:
while n > f:
yield [n, f, f+1, n-1]
f += 2
n -= 2

def test():
for i in range(100):
len_booklet = random.randrange(4, 256, 4)
pages = range(1, random.randint(4, 400))
while pages[-1] < len_booklet:
pages = range(1, random.randint(4, 400))
booklets = list(grouper(pages, len_booklet))
arranged_booklets = list(arrange_booklet(booklets, len_booklet))
book = list(itertools.chain.from_iterable(arranged_booklets))
print("pages : {}\nlen_booklet : {}\n".format(len(pages), len_booklet)
, end='')
if len(book) != len(pages):
print("Error!!!\n", len(book), len(pages))
break
else:
print("All's good\n")

if __name__ == '__main__':
test_or_main = input("Do you want to run a test? (y/n)")
if test_or_main == "n":
print("Running main program...")
pages = range(1, int(input("Enter a number of pages: "))+1)
while True:
len_booklet = int(input(
"Enter booklet length, must be a multiple of four: "))
if len_booklet % 4 == 0 and len_booklet <= len(pages):
break
booklets = list(grouper(pages, len_booklet))
arranged_booklets = list(arrange_booklet(booklets, len_booklet))
book = list(itertools.chain.from_iterable(arranged_booklets))
print(book)
else:
print("Running a test...")
test()