patternpythonMinor
Reorder a list in Python to avoid repeats
Viewed 0 times
reorderrepeatsavoidpythonlist
Problem
I am trying to check if a list has any consecutive repeating elements and then reorder it such that the repeats are avoided. If that is impossible, then return False. For example:
Here is what I have. Is there a more elegant solution?
checkRepeat([1,2])
Out[61]: [1, 2]
checkRepeat([1,2,2])
Out[62]: [2, 1, 2]
checkRepeat([1,2,2,1,1])
Out[63]: [1, 2, 1, 2, 1]
checkRepeat([1,2,2,1,1,3,3,3,3])
Out[64]: [1, 3, 1, 3, 2, 1, 3, 2, 3]
checkRepeat([1,2,2,1,1,3,3,3,3,3])
Out[65]: [3, 1, 3, 2, 3, 1, 3, 1, 3, 2]
checkRepeat([1,2,2,1,1,3,3,3,3,3,3])
Out[66]: [3, 1, 3, 1, 3, 1, 3, 2, 3, 2, 3]
checkRepeat([1,2,2,1,1,3,3,3,3,3,3,3])
Out[67]: FalseHere is what I have. Is there a more elegant solution?
from itertools import groupby
from collections import Counter
def checkRepeat(lst):
"""Returns a list that has no repeating elements. Return False if such a list can't be found"""
def hasRepeat(lst):
"""Returns true if there are any repeats"""
return len([x[0] for x in groupby(lst)]) len(lst)+1: #checks if it will be impossible to find a working order
return False
while hasRepeat(lst):
for i,curElt in enumerate(lst):
try:
if lst[i]==lst[i+1]:
lst[i+1],lst[(i+offset) % len(lst)] = lst[(i+offset) % len(lst)],lst[i+1] #swap j+1 with j+offset. wrap around the list
except:
break
offset+=1
numIter+=1
return lstSolution
Ideally a function will
You should use
You should have spaces either side of your mathermatical operators, it makes them more readable and is recommended by the style guide. So this:
should be this:
Also try to avoid inline comments, they make lines exceedingly long. They should be used sparingly for short comments. If your comment is a sentence it will almost always need its own line. The style guide also says to keep below 79 characters per line, which this comment invalidates:
Split it onto the line above and it's much more readable.
Instead of
What's the point of
Don't use a bare
The line where you swap values is too long and hard to read, instead you should calculate the index of your second value on the line before:
Now that it's clear that
Here's how I'd put the script together:
return a particular type of value. Python may be dynamic and allow you to return anything, but it's less surprising behaviour if you don't return two types of value. Instead, consider returning None or an empty list. Both of which will still evaluate as False when tested because of Python's truthiness. I'd personally suggest None, as an empty list might be mistaken for a useable list. Python will automatically use None as the return value in the absence of any other argument, so just using return returns None.You should use
snake_case for naming variables and functions, so checkRepeat should be check_repeat. I also don't like lst as a name. It's good that you're avoiding shadowing the built in list, but lst is unclear and can look like 1st. Perhaps iterable or source could be better names.hasRepeat doesn't need to be a nested function, you could have it separated as it's own. Having it nested implies that you're trying to do a more complicated structure when really it's just because you call hasRepeat in checkRepeat. Also, checkRepeat isn't a good name as it's actually removing repeating values (if it can). Instead perhaps something like reorder_repeats? It would not be the clearest name because it's not an easy process to describe, but the docstring can provide help there too.You should have spaces either side of your mathermatical operators, it makes them more readable and is recommended by the style guide. So this:
offset=numIter=0should be this:
offset = numIter = 0Also try to avoid inline comments, they make lines exceedingly long. They should be used sparingly for short comments. If your comment is a sentence it will almost always need its own line. The style guide also says to keep below 79 characters per line, which this comment invalidates:
if Counter(lst).most_common()[0][1]*2 > len(lst)+1: #checks if it will be impossible to find a working orderSplit it onto the line above and it's much more readable.
# Check if it will be impossible to find a working order
if Counter(lst).most_common()[0][1] * 2 > len(lst) + 1:Instead of
curElt try the name element or item. Shortened words often make for odd names that people can easily interpret differently to you. Except that you don't end up using this value at all. Instead you access elements by their index, which I personally think is more readable in this case and better, but then you could just loop with this:for i in range(len(source)):What's the point of
numIter? It's the exact same value of offset but never even used? If it was a testing value you should remove it now. While we're on it, I don't entirely understand offset, a comment about it would be helpful when you're incrementing the values.Don't use a bare
except. It's very bad practice because you might ignore all sorts of errors that you should really be paying attention to. In this case, it seems like IndexError is what you're expectin to happen, so use that specifically.The line where you swap values is too long and hard to read, instead you should calculate the index of your second value on the line before:
if lst[i] == lst[i+1]:
index = (i + offset) % len(lst) # Wrap around the list
lst[i+1], lst[index] = lst[index], lst[i+1]Now that it's clear that
index is the same value on either side, it's obvious that you're swapping values. You can make this comment inline since it's a brief note about the meaning of an unusual calculation.Here's how I'd put the script together:
from itertools import groupby
from collections import Counter
def has_repeat(source):
"""Returns true if there are any repeats"""
return len([x[0] for x in groupby(source)]) len(source) + 1:
return
offset = 0
while has_repeat(source):
for i in range(len(source)):
try:
if source[i] == source[i+1]:
index = (i + offset) % len(source) # Wrap around the list
source[i+1], source[index] = source[index], source[i+1]
except IndexError:
break
offset += 1
return sourceCode Snippets
offset=numIter=0offset = numIter = 0if Counter(lst).most_common()[0][1]*2 > len(lst)+1: #checks if it will be impossible to find a working order# Check if it will be impossible to find a working order
if Counter(lst).most_common()[0][1] * 2 > len(lst) + 1:for i in range(len(source)):Context
StackExchange Code Review Q#107630, answer score: 3
Revisions (0)
No revisions yet.