From dd7cd3c15a81ddbcecb42e6f2a8054b24b1f0c4e Mon Sep 17 00:00:00 2001 From: rodferro Date: Sat, 24 Feb 2018 19:45:18 -0300 Subject: [PATCH 1/5] Refactor code Inputs don't need to be sorted, but we have to change how the bottom up implementation populates the results array. We can also simplify some of the statements by: 1) removing the call to list when we create the list 'items', as there's no need for it; 2) initializing the list items with zeros; and 3) removing the if statement in the nested loop, since it's not necessary anymore because of #2. We have to change the test too. We can't hardcode the labels to indices because they may be in a different position. --- .../knapsack_01/knapsack_solution.ipynb | 61 ++++++++----------- .../knapsack_01/test_knapsack.py | 8 +-- 2 files changed, 29 insertions(+), 40 deletions(-) diff --git a/recursion_dynamic/knapsack_01/knapsack_solution.ipynb b/recursion_dynamic/knapsack_01/knapsack_solution.ipynb index aff43197..92266b07 100644 --- a/recursion_dynamic/knapsack_01/knapsack_solution.ipynb +++ b/recursion_dynamic/knapsack_01/knapsack_solution.ipynb @@ -42,7 +42,7 @@ "* Can we assume the inputs are valid?\n", " * No\n", "* Are the inputs in sorted order by val/weight?\n", - " * Yes, if not we'd need to sort them first\n", + " * Yes\n", "* Can we assume this fits memory?\n", " * Yes" ] @@ -132,9 +132,7 @@ { "cell_type": "code", "execution_count": 1, - "metadata": { - "collapsed": true - }, + "metadata": {}, "outputs": [], "source": [ "class Item(object):\n", @@ -158,9 +156,7 @@ { "cell_type": "code", "execution_count": 2, - "metadata": { - "collapsed": false - }, + "metadata": {}, "outputs": [], "source": [ "class Knapsack(object):\n", @@ -170,15 +166,13 @@ " raise TypeError('input_items or total_weight cannot be None')\n", " if not input_items or total_weight == 0:\n", " return 0\n", - " items = list([Item(label='', value=0, weight=0)] + input_items)\n", + " items = [Item(label='', value=0, weight=0)] + input_items\n", " num_rows = len(items)\n", " num_cols = total_weight + 1\n", - " T = [[None] * num_cols for _ in range(num_rows)]\n", + " T = [[0] * num_cols for _ in range(num_rows)]\n", " for i in range(num_rows):\n", " for j in range(num_cols):\n", - " if i == 0 or j == 0:\n", - " T[i][j] = 0\n", - " elif j >= items[i].weight:\n", + " if j >= items[i].weight:\n", " T[i][j] = max(items[i].value + T[i - 1][j - items[i].weight],\n", " T[i - 1][j])\n", " else:\n", @@ -187,14 +181,10 @@ " i = num_rows - 1\n", " j = num_cols - 1\n", " while T[i][j] != 0:\n", - " if T[i - 1][j] == T[i][j]:\n", - " i -= 1\n", - " elif T[i][j - 1] == T[i][j]:\n", - " j -= 1\n", - " else:\n", + " if T[i][j] != T[i - 1][j]:\n", " results.append(items[i])\n", - " i -= 1\n", " j -= items[i].weight\n", + " i -= 1\n", " return results" ] }, @@ -208,9 +198,7 @@ { "cell_type": "code", "execution_count": 3, - "metadata": { - "collapsed": false - }, + "metadata": {}, "outputs": [], "source": [ "class KnapsackTopDown(object):\n", @@ -255,9 +243,7 @@ { "cell_type": "code", "execution_count": 4, - "metadata": { - "collapsed": false - }, + "metadata": {}, "outputs": [], "source": [ "class Result(object):\n", @@ -328,9 +314,7 @@ { "cell_type": "code", "execution_count": 5, - "metadata": { - "collapsed": false - }, + "metadata": {}, "outputs": [ { "name": "stdout", @@ -342,8 +326,7 @@ ], "source": [ "%%writefile test_knapsack.py\n", - "from nose.tools import assert_equal, assert_raises\n", - "\n", + "from nose.tools import assert_equal, assert_raises, assert_true\n", "\n", "class TestKnapsack(object):\n", "\n", @@ -359,8 +342,9 @@ " total_weight = 8\n", " expected_value = 13\n", " results = knapsack.fill_knapsack(items, total_weight)\n", - " assert_equal(results[0].label, 'd')\n", - " assert_equal(results[1].label, 'b')\n", + " labels = [x.label for x in results]\n", + " assert_true('b' in labels)\n", + " assert_true('d' in labels)\n", " total_value = 0\n", " for item in results:\n", " total_value += item.value\n", @@ -394,9 +378,7 @@ { "cell_type": "code", "execution_count": 6, - "metadata": { - "collapsed": false - }, + "metadata": {}, "outputs": [ { "name": "stdout", @@ -410,6 +392,13 @@ "source": [ "%run -i test_knapsack.py" ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [] } ], "metadata": { @@ -428,9 +417,9 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.4.3" + "version": "3.6.4" } }, "nbformat": 4, - "nbformat_minor": 0 + "nbformat_minor": 1 } diff --git a/recursion_dynamic/knapsack_01/test_knapsack.py b/recursion_dynamic/knapsack_01/test_knapsack.py index 5388b1be..625b5324 100644 --- a/recursion_dynamic/knapsack_01/test_knapsack.py +++ b/recursion_dynamic/knapsack_01/test_knapsack.py @@ -1,5 +1,4 @@ -from nose.tools import assert_equal, assert_raises - +from nose.tools import assert_equal, assert_raises, assert_true class TestKnapsack(object): @@ -15,8 +14,9 @@ def test_knapsack_bottom_up(self): total_weight = 8 expected_value = 13 results = knapsack.fill_knapsack(items, total_weight) - assert_equal(results[0].label, 'd') - assert_equal(results[1].label, 'b') + labels = [x.label for x in results] + assert_true('b' in labels) + assert_true('d' in labels) total_value = 0 for item in results: total_value += item.value From cdf93bdec4bb5972b511fc035c4b1d13a6918143 Mon Sep 17 00:00:00 2001 From: rodferro Date: Fri, 2 Mar 2018 14:40:18 -0300 Subject: [PATCH 2/5] Fix a potential bug Range has to start at 1, otherwise, during the first row iteration, we're going to get values from the last row of table T (T[-1]). --- .../knapsack_01/knapsack_solution.ipynb | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/recursion_dynamic/knapsack_01/knapsack_solution.ipynb b/recursion_dynamic/knapsack_01/knapsack_solution.ipynb index 92266b07..01118ec3 100644 --- a/recursion_dynamic/knapsack_01/knapsack_solution.ipynb +++ b/recursion_dynamic/knapsack_01/knapsack_solution.ipynb @@ -132,7 +132,9 @@ { "cell_type": "code", "execution_count": 1, - "metadata": {}, + "metadata": { + "collapsed": true + }, "outputs": [], "source": [ "class Item(object):\n", @@ -156,7 +158,9 @@ { "cell_type": "code", "execution_count": 2, - "metadata": {}, + "metadata": { + "collapsed": true + }, "outputs": [], "source": [ "class Knapsack(object):\n", @@ -170,8 +174,8 @@ " num_rows = len(items)\n", " num_cols = total_weight + 1\n", " T = [[0] * num_cols for _ in range(num_rows)]\n", - " for i in range(num_rows):\n", - " for j in range(num_cols):\n", + " for i in range(1, num_rows):\n", + " for j in range(1, num_cols):\n", " if j >= items[i].weight:\n", " T[i][j] = max(items[i].value + T[i - 1][j - items[i].weight],\n", " T[i - 1][j])\n", @@ -198,7 +202,9 @@ { "cell_type": "code", "execution_count": 3, - "metadata": {}, + "metadata": { + "collapsed": true + }, "outputs": [], "source": [ "class KnapsackTopDown(object):\n", @@ -243,7 +249,9 @@ { "cell_type": "code", "execution_count": 4, - "metadata": {}, + "metadata": { + "collapsed": true + }, "outputs": [], "source": [ "class Result(object):\n", @@ -396,7 +404,9 @@ { "cell_type": "code", "execution_count": null, - "metadata": {}, + "metadata": { + "collapsed": true + }, "outputs": [], "source": [] } @@ -417,7 +427,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.6.4" + "version": "3.6.3" } }, "nbformat": 4, From 37bb7833f76f73a4230f730bb3f3bdaeed9f7492 Mon Sep 17 00:00:00 2001 From: rodferro Date: Sat, 3 Mar 2018 21:17:27 -0300 Subject: [PATCH 3/5] Refactor insert method We can simplify the insert method by removing the nested conditionals. --- graphs_trees/bst/bst.py | 17 +++------ graphs_trees/bst/bst_solution.ipynb | 53 ++++++++--------------------- 2 files changed, 20 insertions(+), 50 deletions(-) diff --git a/graphs_trees/bst/bst.py b/graphs_trees/bst/bst.py index 9b23e8e2..93907068 100644 --- a/graphs_trees/bst/bst.py +++ b/graphs_trees/bst/bst.py @@ -28,16 +28,9 @@ def _insert(self, node, data): if node is None: return Node(data) if data <= node.data: - if node.left is None: - node.left = self._insert(node.left, data) - node.left.parent = node - return node.left - else: - return self._insert(node.left, data) + node.left = self._insert(node.left, data) + node.left.parent = node else: - if node.right is None: - node.right = self._insert(node.right, data) - node.right.parent = node - return node.right - else: - return self._insert(node.right, data) \ No newline at end of file + node.right = self._insert(node.right, data) + node.right.parent = node + return node \ No newline at end of file diff --git a/graphs_trees/bst/bst_solution.ipynb b/graphs_trees/bst/bst_solution.ipynb index fb3bc5a9..d770e562 100644 --- a/graphs_trees/bst/bst_solution.ipynb +++ b/graphs_trees/bst/bst_solution.ipynb @@ -74,12 +74,8 @@ "### Insert\n", "\n", "* If the root is None, return Node(data)\n", - "* If the data is <= the current node's data\n", - " * If the current node's left child is None, set it to Node(data)\n", - " * Else, recursively call insert on the left child\n", - "* Else\n", - " * If the current node's right child is None, set it to Node(data)\n", - " * Else, recursively call insert on the right child\n", + "* If the data is <= the current node's data, recursively call insert on the left child\n", + "* Else, recursively call insert on the right child\n", "\n", "Complexity:\n", "\n", @@ -99,9 +95,7 @@ { "cell_type": "code", "execution_count": 1, - "metadata": { - "collapsed": false - }, + "metadata": {}, "outputs": [ { "name": "stdout", @@ -143,27 +137,18 @@ " if node is None:\n", " return Node(data)\n", " if data <= node.data:\n", - " if node.left is None:\n", - " node.left = self._insert(node.left, data)\n", - " node.left.parent = node\n", - " return node.left\n", - " else:\n", - " return self._insert(node.left, data)\n", + " node.left = self._insert(node.left, data)\n", + " node.left.parent = node\n", " else:\n", - " if node.right is None:\n", - " node.right = self._insert(node.right, data)\n", - " node.right.parent = node\n", - " return node.right\n", - " else:\n", - " return self._insert(node.right, data)" + " node.right = self._insert(node.right, data)\n", + " node.right.parent = node\n", + " return node" ] }, { "cell_type": "code", "execution_count": 2, - "metadata": { - "collapsed": false - }, + "metadata": {}, "outputs": [], "source": [ "%run bst.py" @@ -179,9 +164,7 @@ { "cell_type": "code", "execution_count": 3, - "metadata": { - "collapsed": true - }, + "metadata": {}, "outputs": [], "source": [ "%run dfs.py" @@ -190,9 +173,7 @@ { "cell_type": "code", "execution_count": 4, - "metadata": { - "collapsed": false - }, + "metadata": {}, "outputs": [], "source": [ "%run ../utils/results.py" @@ -201,9 +182,7 @@ { "cell_type": "code", "execution_count": 5, - "metadata": { - "collapsed": false - }, + "metadata": {}, "outputs": [ { "name": "stdout", @@ -260,9 +239,7 @@ { "cell_type": "code", "execution_count": 6, - "metadata": { - "collapsed": false - }, + "metadata": {}, "outputs": [ { "name": "stdout", @@ -293,9 +270,9 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.4.3" + "version": "3.6.4" } }, "nbformat": 4, - "nbformat_minor": 0 + "nbformat_minor": 1 } From 0110698caae30f13e3d8d287341c62486c623e13 Mon Sep 17 00:00:00 2001 From: rodferro Date: Sat, 3 Mar 2018 21:31:22 -0300 Subject: [PATCH 4/5] Revert "Refactor insert method" This reverts commit 37bb7833f76f73a4230f730bb3f3bdaeed9f7492. --- graphs_trees/bst/bst.py | 17 ++++++--- graphs_trees/bst/bst_solution.ipynb | 53 +++++++++++++++++++++-------- 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/graphs_trees/bst/bst.py b/graphs_trees/bst/bst.py index 93907068..9b23e8e2 100644 --- a/graphs_trees/bst/bst.py +++ b/graphs_trees/bst/bst.py @@ -28,9 +28,16 @@ def _insert(self, node, data): if node is None: return Node(data) if data <= node.data: - node.left = self._insert(node.left, data) - node.left.parent = node + if node.left is None: + node.left = self._insert(node.left, data) + node.left.parent = node + return node.left + else: + return self._insert(node.left, data) else: - node.right = self._insert(node.right, data) - node.right.parent = node - return node \ No newline at end of file + if node.right is None: + node.right = self._insert(node.right, data) + node.right.parent = node + return node.right + else: + return self._insert(node.right, data) \ No newline at end of file diff --git a/graphs_trees/bst/bst_solution.ipynb b/graphs_trees/bst/bst_solution.ipynb index d770e562..fb3bc5a9 100644 --- a/graphs_trees/bst/bst_solution.ipynb +++ b/graphs_trees/bst/bst_solution.ipynb @@ -74,8 +74,12 @@ "### Insert\n", "\n", "* If the root is None, return Node(data)\n", - "* If the data is <= the current node's data, recursively call insert on the left child\n", - "* Else, recursively call insert on the right child\n", + "* If the data is <= the current node's data\n", + " * If the current node's left child is None, set it to Node(data)\n", + " * Else, recursively call insert on the left child\n", + "* Else\n", + " * If the current node's right child is None, set it to Node(data)\n", + " * Else, recursively call insert on the right child\n", "\n", "Complexity:\n", "\n", @@ -95,7 +99,9 @@ { "cell_type": "code", "execution_count": 1, - "metadata": {}, + "metadata": { + "collapsed": false + }, "outputs": [ { "name": "stdout", @@ -137,18 +143,27 @@ " if node is None:\n", " return Node(data)\n", " if data <= node.data:\n", - " node.left = self._insert(node.left, data)\n", - " node.left.parent = node\n", + " if node.left is None:\n", + " node.left = self._insert(node.left, data)\n", + " node.left.parent = node\n", + " return node.left\n", + " else:\n", + " return self._insert(node.left, data)\n", " else:\n", - " node.right = self._insert(node.right, data)\n", - " node.right.parent = node\n", - " return node" + " if node.right is None:\n", + " node.right = self._insert(node.right, data)\n", + " node.right.parent = node\n", + " return node.right\n", + " else:\n", + " return self._insert(node.right, data)" ] }, { "cell_type": "code", "execution_count": 2, - "metadata": {}, + "metadata": { + "collapsed": false + }, "outputs": [], "source": [ "%run bst.py" @@ -164,7 +179,9 @@ { "cell_type": "code", "execution_count": 3, - "metadata": {}, + "metadata": { + "collapsed": true + }, "outputs": [], "source": [ "%run dfs.py" @@ -173,7 +190,9 @@ { "cell_type": "code", "execution_count": 4, - "metadata": {}, + "metadata": { + "collapsed": false + }, "outputs": [], "source": [ "%run ../utils/results.py" @@ -182,7 +201,9 @@ { "cell_type": "code", "execution_count": 5, - "metadata": {}, + "metadata": { + "collapsed": false + }, "outputs": [ { "name": "stdout", @@ -239,7 +260,9 @@ { "cell_type": "code", "execution_count": 6, - "metadata": {}, + "metadata": { + "collapsed": false + }, "outputs": [ { "name": "stdout", @@ -270,9 +293,9 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.6.4" + "version": "3.4.3" } }, "nbformat": 4, - "nbformat_minor": 1 + "nbformat_minor": 0 } From 2e9ceb57051ef9e311ba676996efb5494ca3723f Mon Sep 17 00:00:00 2001 From: rodferro Date: Sat, 3 Mar 2018 21:47:48 -0300 Subject: [PATCH 5/5] Update authors --- recursion_dynamic/knapsack_01/knapsack_solution.ipynb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/recursion_dynamic/knapsack_01/knapsack_solution.ipynb b/recursion_dynamic/knapsack_01/knapsack_solution.ipynb index 01118ec3..57b527d9 100644 --- a/recursion_dynamic/knapsack_01/knapsack_solution.ipynb +++ b/recursion_dynamic/knapsack_01/knapsack_solution.ipynb @@ -4,7 +4,7 @@ "cell_type": "markdown", "metadata": {}, "source": [ - "This notebook was prepared by [Donne Martin](https://github.com/donnemartin). Source and license info is on [GitHub](https://github.com/donnemartin/interactive-coding-challenges)." + "This notebook was prepared by [Donne Martin](https://github.com/donnemartin) and [Rodrigo Ferro](https://github.com/rodferro). Source and license info is on [GitHub](https://github.com/donnemartin/interactive-coding-challenges)." ] }, { @@ -427,7 +427,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.6.3" + "version": "3.6.4" } }, "nbformat": 4,