Repository: openaddresses/machine
Pull request #498 (2016-12-17 05:53:10)

Fix remove_prefix and remove_postfix functions
`remove_postfix` has a bug where a zero-length `field_to_remove` value was causing the entire `field` to be assigned an empty string instead of not modifying the value.  

Also added zero-length `field_to_remove` value test to `remove_prefix` for thoroughness.  


Patch:
diff --git a/openaddr/conform.py b/openaddr/conform.py
index 30ca000..e2bf976 100644
--- a/openaddr/conform.py
+++ b/openaddr/conform.py
@@ -999,7 +999,7 @@ def row_fxn_remove_postfix(sd, row, key):
     "Remove a 'field_to_remove' from the end of 'field' if it is a postfix"
     fxn = sd["conform"][key]
 
-    if row[fxn["field"]].endswith(row[fxn["field_to_remove"]]):
+    if row[fxn["field_to_remove"]] != "" and row[fxn["field"]].endswith(row[fxn["field_to_remove"]]):
         row[attrib_types[key]] = row[fxn["field"]][0:len(row[fxn["field_to_remove"]])*-1].rstrip(' ')
     else:
         row[attrib_types[key]] = row[fxn["field"]]


Test patch:
diff --git a/openaddr/tests/conform.py b/openaddr/tests/conform.py
index 91de8ff..606cb62 100644
--- a/openaddr/tests/conform.py
+++ b/openaddr/tests/conform.py
@@ -372,68 +372,6 @@ class TestConformTransforms (unittest.TestCase):
         d = row_fxn_postfixed_street(c, d, "street")
         self.assertEqual(e, d)
     
-    def test_row_fxn_remove_prefix(self):
-        "remove_prefix - field_to_remove is a prefix"
-        c = { "conform": {
-            "street": {
-                "function": "remove_prefix",
-                "field": "ADDRESS",
-                "field_to_remove": "PREFIX"
-            }
-        } }
-        d = { "ADDRESS": "123 MAPLE ST", "PREFIX": "123" }
-        e = copy.deepcopy(d)
-        e.update({ "OA:street": "MAPLE ST" })
-
-        d = row_fxn_remove_prefix(c, d, "street")
-        self.assertEqual(e, d)
-
-        "remove_prefix - field_to_remove is not a prefix"
-        c = { "conform": {
-            "street": {
-                "function": "remove_prefix",
-                "field": "ADDRESS",
-                "field_to_remove": "PREFIX"
-            }
-        } }
-        d = { "ADDRESS": "123 MAPLE ST", "PREFIX": "NOT THE PREFIX VALUE" }
-        e = copy.deepcopy(d)
-        e.update({ "OA:street": "123 MAPLE ST" })
-
-        d = row_fxn_remove_prefix(c, d, "street")
-        self.assertEqual(e, d)
-
-    def test_row_fxn_remove_postfix(self):
-        "remove_postfix - field_to_remove is a postfix"
-        c = { "conform": {
-            "street": {
-                "function": "remove_postfix",
-                "field": "ADDRESS",
-                "field_to_remove": "POSTFIX"
-            }
-        } }
-        d = { "ADDRESS": "MAPLE ST UNIT 5", "POSTFIX": "UNIT 5" }
-        e = copy.deepcopy(d)
-        e.update({ "OA:street": "MAPLE ST" })
-
-        d = row_fxn_remove_postfix(c, d, "street")
-        self.assertEqual(e, d)
-
-        "remove_postfix - field_to_remove is not a postfix"
-        c = { "conform": {
-            "street": {
-                "function": "remove_postfix",
-                "field": "ADDRESS",
-                "field_to_remove": "POSTFIX"
-            }
-        } }
-        d = { "ADDRESS": "123 MAPLE ST", "POSTFIX": "NOT THE POSTFIX VALUE" }
-        e = copy.deepcopy(d)
-        e.update({ "OA:street": "123 MAPLE ST" })
-
-        d = row_fxn_remove_postfix(c, d, "street")
-        self.assertEqual(e, d)
-
         "Regex prefixed_number and postfixed_number - ordinal street w/house number"
         c = { "conform": {
             "number": {
@@ -548,6 +486,98 @@ class TestConformTransforms (unittest.TestCase):
         d = row_fxn_postfixed_street(c, d, "street")
         self.assertEqual(e, d)
 
+    def test_row_fxn_remove_prefix(self):
+        "remove_prefix - field_to_remove is a prefix"
+        c = { "conform": {
+            "street": {
+                "function": "remove_prefix",
+                "field": "ADDRESS",
+                "field_to_remove": "PREFIX"
+            }
+        } }
+        d = { "ADDRESS": "123 MAPLE ST", "PREFIX": "123" }
+        e = copy.deepcopy(d)
+        e.update({ "OA:street": "MAPLE ST" })
+
+        d = row_fxn_remove_prefix(c, d, "street")
+        self.assertEqual(e, d)
+
+        "remove_prefix - field_to_remove is not a prefix"
+        c = { "conform": {
+            "street": {
+                "function": "remove_prefix",
+                "field": "ADDRESS",
+                "field_to_remove": "PREFIX"
+            }
+        } }
+        d = { "ADDRESS": "123 MAPLE ST", "PREFIX": "NOT THE PREFIX VALUE" }
+        e = copy.deepcopy(d)
+        e.update({ "OA:street": "123 MAPLE ST" })
+
+        d = row_fxn_remove_prefix(c, d, "street")
+        self.assertEqual(e, d)
+
+        "remove_prefix - field_to_remove value is empty string"
+        c = { "conform": {
+            "street": {
+                "function": "remove_prefix",
+                "field": "ADDRESS",
+                "field_to_remove": "PREFIX"
+            }
+        } }
+        d = { "ADDRESS": "123 MAPLE ST", "PREFIX": "" }
+        e = copy.deepcopy(d)
+        e.update({ "OA:street": "123 MAPLE ST" })
+
+        d = row_fxn_remove_prefix(c, d, "street")
+        self.assertEqual(e, d)
+
+    def test_row_fxn_remove_postfix(self):
+        "remove_postfix - field_to_remove is a postfix"
+        c = { "conform": {
+            "street": {
+                "function": "remove_postfix",
+                "field": "ADDRESS",
+                "field_to_remove": "POSTFIX"
+            }
+        } }
+        d = { "ADDRESS": "MAPLE ST UNIT 5", "POSTFIX": "UNIT 5" }
+        e = copy.deepcopy(d)
+        e.update({ "OA:street": "MAPLE ST" })
+
+        d = row_fxn_remove_postfix(c, d, "street")
+        self.assertEqual(e, d)
+
+        "remove_postfix - field_to_remove is not a postfix"
+        c = { "conform": {
+            "street": {
+                "function": "remove_postfix",
+                "field": "ADDRESS",
+                "field_to_remove": "POSTFIX"
+            }
+        } }
+        d = { "ADDRESS": "123 MAPLE ST", "POSTFIX": "NOT THE POSTFIX VALUE" }
+        e = copy.deepcopy(d)
+        e.update({ "OA:street": "123 MAPLE ST" })
+
+        d = row_fxn_remove_postfix(c, d, "street")
+        self.assertEqual(e, d)
+
+        "remove_postfix - field_to_remove value is empty string"
+        c = { "conform": {
+            "street": {
+                "function": "remove_postfix",
+                "field": "ADDRESS",
+                "field_to_remove": "POSTFIX"
+            }
+        } }
+        d = { "ADDRESS": "123 MAPLE ST", "POSTFIX": "" }
+        e = copy.deepcopy(d)
+        e.update({ "OA:street": "123 MAPLE ST" })
+
+        d = row_fxn_remove_postfix(c, d, "street")
+        self.assertEqual(e, d)
+
 class TestConformCli (unittest.TestCase):
     "Test the command line interface creates valid output files from test input"
     def setUp(self):