Long Refactoring: pep8

While writing the last article, I noticed that I had made a bunch of little changes to the files without explicitly meaning to. I didn’t include them in the diffs. However, on my screen, I have a bunch of changes that appear to be…not changes:

                     current_node.next()
                 else:
                     current_node.next()
-            
+
         return_string = ''

This is due to me running the whitespace cleanup on the code base: I noticed that I had introduced some white space while editing the file, and it shows up in the diffs. It has become an ingrained habit to clean those up.

Since I fixed that, I am also going to fix the rest of the pep8 errors reported by the flake8 tool.

After running the emacs command white-space cleanup, a bunch of the tox errors go away. Here is the tail end of what I am left with:

./treesudoku/tree_sudoku.py:154:80: E501 line too long (84 > 79 characters)
                    box_list.append(str(box_center[0] + i) + str(box_center[1] + 1))
                                                                               ^
./treesudoku/tree_sudoku.py:169:1: E302 expected 2 blank lines, found 1
class Tree_Node:
^

When fixing these errors, I start at the bottom of the file and work my way to the top. Why? so the line numbers stay consistent.

pep8 is pedantic about spacing. Non class functions should be exactly two lines apart. Methods of a class should be one line apart. The tool tells you where this in not met. Start by adding an additional newline on line 169. Make sure you don’t introduce whitespace

Long lines are a pain. Making long lines shorter is a different kind of pain. There are several techniques. For python, the most important is to know that you can wrap at open parenthesis, brackets, and braces. Change line 154 to two lines like this.

-                    box_list.append(str(box_center[0] + i) + str(box_center[1] + 1))
+                    box_list.append(
+                        str(box_center[0] + i) + str(box_center[1] + 1))

Go ahead and do that for the two lines above it as well….they are all guilty. Similar change as line 122

-        box_indexes = self.box_index_table[self.find_box_of_index(str(row_index) + str(column_index))]
+        box_indexes = self.box_index_table[
+            self.find_box_of_index(str(row_index) + str(column_index))]

pep8 is also strict about spacing around various keywords and tokens. During definition and on stand alone lines it wants spaces around and equals sign, but during function calls and headers it does not. Change line 96 like this:

-                    print('| ', end = '')
+                    print('| ', end='')

Now we come to this pair of lines that are too long:

            if test_board[int(new_node.board_spot[0])][int(new_node.board_spot[1])] != '0':
                    new_node.value = test_board[int(new_node.board_spot[0])][int(new_node.board_spot[1])]

The code formatter does not do us much good here. The easiest way to shorten these lines is to introduce a shorter variable for the values inside the brackets. Since this is a code change, we’ll want to run the unit tests before and after.

I am going to name these variables row and col, short for column.

-                if test_board[int(new_node.board_spot[0])][int(new_node.board_spot[1])] != '0':
-                    new_node.value = test_board[int(new_node.board_spot[0])][int(new_node.board_spot[1])]
+
+                row = int(new_node.board_spot[0])
+                col = int(new_node.board_spot[1])
+                if test_board[row][col] != '0':
+                    new_node.value = test_board[row][col]

The next few changes are long lines, and we can apply one of the previous techniques for each. But be careful at the change on line 51. The above section is indented (it is in a while loop) and we do not want to indent our variables inside the loop. If the while loop is never executed, our variables will be undefined.

I also note that we can start getting confused between variables. I am going to name this pair curr_row and curr_col, since they come from the int(current_node.board_spot[0])

-            if self.value_valid(test_board, int(current_node.board_spot[0]), int(current_node.board_spot[1])):
+
+            curr_row = int(current_node.board_spot[0])
+            curr_col = int(current_node.board_spot[1])
+            if self.value_valid(test_board, curr_row, curr_col):

For the long line 80 I use the short names of row and col to avoid spilling over even after extracting. Long variables names and deep indentation can make this task bothersome. Line 47 is too long, and none of our previous tricks will work. I am going to change the string current_ to curr_ in this whole function, and it will shorten this line by 6 characters, just enough.

Line 45 is another extract variable shortening.

The final change for this file is the comment block at the top. Insert a space after the hash mark on each line:

@@ -1,8 +1,8 @@
-#turn each csv row into a board
-#find what values can go into what spot
-#create a tree trying to put in each value
-#if value can not be possible, end stem, go back up the tree
-#retrun the branch when tree is MAX=81 layers deep, the board is filled
+# turn each csv row into a board
+# find what values can go into what spot
+# create a tree trying to put in each value
+# if value can not be possible, end stem, go back up the tree
+# return the branch when tree is MAX=81 layers deep, the board is filled

That leaves the code in the test_tree_soduku file. We’ll ignore the typo in the name for now, and just shorten the strings. Use the fact that strings can be added together and defined within parenthesis, and we get a style that works like this:

 puzzles = {
-    "0": "483921657967345821251876493548132976729564138136798245372689514814253769695417382",
-    "1": "245981376169273584837564219976125438513498627482736951391657842728349165654812793",
-    "2": "462831957795426183381795426173984265659312748248567319926178534834259671517643892"
+    "0": ("483921657" +
+          "967345821" +
+          "251876493" +
+          "548132976" +
+          "729564138" +
+          "136798245" +
+          "372689514" +
+          "814253769" +
+          "695417382"),
+    "1": ("245981376" +
+          "169273584" +
+          "837564219" +
+          "976125438" +
+          "513498627" +
+          "482736951" +
+          "391657842" +
+          "728349165" +
+          "654812793"),
+    "2": ("462831957" +
+          "795426183" +
+          "381795426" +
+          "173984265" +
+          "659312748" +
+          "248567319" +
+          "926178534" +
+          "834259671" +
+          "517643892")
 }

Note that this makes the puzzles readable to the human eye, as they are in the standard 9 X 9 layout.

We have one error left:

./treesudoku/test_tree_soduku.py:1:1: F401 'subprocess' imported but unused
import subprocess

This is the kind of thing we really want the flake8 check for. We now know we can remove this import.

@@ -1,11 +1,33 @@
-import subprocess
-

and now tox reports

===================================================================== 1 passed in 13.43s =====================================================================
__________________________________________________________________________ summary ___________________________________________________________________________
  pep8: commands succeeded
  py37: commands succeeded
  congratulations :)

Commit to git and take a break.

Next up: Extracting the helper functions.

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.