Return to Answer

Added code review

Source Link

edited Aug 22, 2018 at 22:41

Note: Using ast.literal_eval() is about 4 times slower than Graipher's method, but it handles general python literals, including strings, and dictionaries in addition to lists of tuples. With great power comes reduced speed.

Code review comments on your implementation

(Excluding what Graipher has already mentioned)

You can use slices to get selected portions of lists. Consider l_data. After .split()-ing, you use .pop() to get rid of the last element.

l_data = new_data.split("),")
l_data.pop()

You can split the data and get rid of the last element in one statement, using the slice [:-1]. The slice :-1 translates to "all elements excluding the last one".

l_data = new_data.split("),")[:-1]

The statement l_work = [] is unnecessary. l_work = i.split(",") will overwrite the initialized list.

Initializing l_nbrs = [] outside your loop is unnecessary, and requires that you reinitialize it at the end of the loop. Instead, move it inside the loop, at the top:

for i in l_data:
    l_work = i.split(",")
    l_nbrs = []            # Initialized here
    for j in l_work:
        l_nbrs.append(int(j))
    l_all.append(l_nbrs)

This is referred to lately as keeping your code DRY (Don't Repeat Yourself), in contrast to WET (Write Everything Twice) code.

List comprehension is a powerful tool. The inner loop can be replaced with the following single statement:

    l_nbrs = [ int(j) for j in l_work ]

Once you understand that, the code can be reduced further by avoiding the single use l_work and l_nbrs variables::

for i in l_data:
    l_all.append( [ int(j) for j in i.split(",") ] )

Code review comments on your implementation

(Excluding what Graipher has already mentioned)

You can use slices to get selected portions of lists. Consider l_data. After .split()-ing, you use .pop() to get rid of the last element.

l_data = new_data.split("),")
l_data.pop()

You can split the data and get rid of the last element in one statement, using the slice [:-1]. The slice :-1 translates to "all elements excluding the last one".

l_data = new_data.split("),")[:-1]

The statement l_work = [] is unnecessary. l_work = i.split(",") will overwrite the initialized list.

Initializing l_nbrs = [] outside your loop is unnecessary, and requires that you reinitialize it at the end of the loop. Instead, move it inside the loop, at the top:

for i in l_data:
    l_work = i.split(",")
    l_nbrs = []            # Initialized here
    for j in l_work:
        l_nbrs.append(int(j))
    l_all.append(l_nbrs)

This is referred to lately as keeping your code DRY (Don't Repeat Yourself), in contrast to WET (Write Everything Twice) code.

List comprehension is a powerful tool. The inner loop can be replaced with the following single statement:

    l_nbrs = [ int(j) for j in l_work ]

Once you understand that, the code can be reduced further by avoiding the single use l_work and l_nbrs variables::

for i in l_data:
    l_all.append( [ int(j) for j in i.split(",") ] )

Source Link

created Aug 22, 2018 at 22:09

AJNeufeld

The Abstract Syntax Trees module is perfect for converting string literals into python values, such as lists and tuples. Specifically, the ast.literal_eval() method in AST Helpers.

def str_to_nbrs(data):
    return ast.literal_eval("[" + data + "]")

>>> str_to_nbrs("(2,2),(6,2),(2,6)")

[(2, 2), (6, 2), (2, 6)]

Surrounding the string "(2,2),(6,2),(2,6)" with [ and ] characters will turn it into the literal string for a list, which is necessary to ensure the result is a nested list of tuples.

ast.literal_eval("[(2,2),(6,2),(2,6)]")  # [(2,2),(6,2),(2,6)]  list of tuples
ast.literal_eval("(2,2),(6,2),(2,6)")    # ((2,2),(6,2),(2,6))  tuple of tuples
ast.literal_eval("[(2,2)]")              # [(2,2)]              list of 1 tuple
ast.literal_eval("(2,2)")                # (2,2)                Oops! Not a tuple of tuples!

Note: You can surround the string with with ( ), but "((2,2))" still evaluates as the tuple (2,2). You need the trailing comma "((2,2),)" in order to convert to the nested tuple of exactly 1 tuple: ((2,2),)