further optimize normal_form

Avoid unnecessary splitting and joining of the rows. The current implementation would split the file into rows in each of the is_form_x separately. They all would do it the same way.

So instead, we can split the file once and pass the lines to the is_form_x directly. It also allows us to avoid "re-joining" of the lines in is_form_5 when it calls the is_form_2.

The test_normal_forms test inputs were changed accordingly: they are split by the `\n` and the trailing newlines were manually removed (the actual code will always strip the trailing newlines before calling the is_form_x functions).

Author: no23reason

clevercsv/normal_form.py

@@ -62,7 +62,7 @@ def detect_dialect_normal(
             return None
 
     form_and_dialect: List[
-        Tuple[int, Callable[[str, SimpleDialect], bool], SimpleDialect]
+        Tuple[int, Callable[[list[str], SimpleDialect], bool], SimpleDialect]
     ] = []
 
     for delim in delimiters:
@@ -90,8 +90,10 @@ def detect_dialect_normal(
         )
     )
 
+    rows = split_file(data)
+
     for ID, form_func, dialect in form_and_dialect:
-        if form_func(data, dialect):
+        if form_func(rows, dialect):
             if verbose:
                 print("Matched normal form %i." % ID)
             return dialect
@@ -228,13 +230,11 @@ def split_row(row: str, dialect: SimpleDialect) -> List[str]:
     return cells
 
 
-def is_form_1(data: str, dialect: SimpleDialect) -> bool:
+def is_form_1(rows: list[str], dialect: SimpleDialect) -> bool:
     # All cells quoted, quoted empty allowed, no nested quotes, more than one
     # column
     assert dialect.quotechar is not None
 
-    rows = split_file(data)
-
     if not every_row_has_delim_and_is_the_same_length(rows, dialect):
         return False
 
@@ -258,11 +258,8 @@ def is_form_1(data: str, dialect: SimpleDialect) -> bool:
     return True
 
 
-def is_form_2(data: str, dialect: SimpleDialect) -> bool:
+def is_form_2(rows: list[str], dialect: SimpleDialect) -> bool:
     # All unquoted, empty allowed, all elementary
-
-    rows = split_file(data)
-
     if not every_row_has_delim_and_is_the_same_length(rows, dialect):
         return False
 
@@ -283,12 +280,10 @@ def is_form_2(data: str, dialect: SimpleDialect) -> bool:
     return True
 
 
-def is_form_3(data: str, dialect: SimpleDialect) -> bool:
+def is_form_3(rows: list[str], dialect: SimpleDialect) -> bool:
     # some quoted, some not quoted, no empty, no nested quotes
     assert dialect.quotechar is not None
 
-    rows = split_file(data)
-
     if not every_row_has_delim_and_is_the_same_length(rows, dialect):
         return False
     if len(rows) <= 1:
@@ -318,12 +313,10 @@ def is_form_3(data: str, dialect: SimpleDialect) -> bool:
     return True
 
 
-def is_form_4(data: str, dialect: SimpleDialect) -> bool:
+def is_form_4(rows: list[str], dialect: SimpleDialect) -> bool:
     # no delim, single column (either entirely quoted or entirely unquoted)
     assert dialect.quotechar is not None
 
-    rows = split_file(data)
-
     if len(rows) <= 1:
         return False
 
@@ -345,12 +338,9 @@ def is_form_4(data: str, dialect: SimpleDialect) -> bool:
     return True
 
 
-def is_form_5(data: str, dialect: SimpleDialect) -> bool:
+def is_form_5(rows: list[str], dialect: SimpleDialect) -> bool:
     # all rows quoted, no nested quotes
     # basically form 2 but with quotes around each row
-
-    rows = split_file(data)
-
     if not every_row_has_delim(rows, dialect):
         return False
     if len(rows) <= 1:
@@ -368,4 +358,4 @@ def is_form_5(data: str, dialect: SimpleDialect) -> bool:
     for row in rows:
         newrows.append(row[1:-1])
 
-    return is_form_2("\n".join(newrows), dialect)
+    return is_form_2(newrows, dialect)

tests/test_unit/test_normal_forms.py

@@ -21,71 +21,73 @@ class NormalFormTestCase(unittest.TestCase):
     def test_form_1(self) -> None:
         dialect = SimpleDialect(delimiter=",", quotechar='"', escapechar="")
 
-        self.assertTrue(is_form_1('"A","B","C"', dialect))
-        self.assertTrue(is_form_1('"A","B"\n"C","D"\n', dialect))
-        self.assertTrue(is_form_1('"A","","C"', dialect))
+        self.assertTrue(is_form_1('"A","B","C"'.split("\n"), dialect))
+        self.assertTrue(is_form_1('"A","B"\n"C","D"'.split("\n"), dialect))
+        self.assertTrue(is_form_1('"A","","C"'.split("\n"), dialect))
 
-        self.assertFalse(is_form_1('"A","B"\n"A"', dialect))
-        self.assertFalse(is_form_1('"A"\n"B"', dialect))
-        self.assertFalse(is_form_1('"A"\n"A","B"', dialect))
-        self.assertFalse(is_form_1('"A",,"C"', dialect))
-        self.assertFalse(is_form_1('"A",C', dialect))
-        self.assertFalse(is_form_1('"A"\n"b""A""c","B"', dialect))
+        self.assertFalse(is_form_1('"A","B"\n"A"'.split("\n"), dialect))
+        self.assertFalse(is_form_1('"A"\n"B"'.split("\n"), dialect))
+        self.assertFalse(is_form_1('"A"\n"A","B"'.split("\n"), dialect))
+        self.assertFalse(is_form_1('"A",,"C"'.split("\n"), dialect))
+        self.assertFalse(is_form_1('"A",C'.split("\n"), dialect))
+        self.assertFalse(is_form_1('"A"\n"b""A""c","B"'.split("\n"), dialect))
 
     def test_form_2(self) -> None:
         dialect = SimpleDialect(delimiter=",", quotechar="", escapechar="")
 
-        self.assertTrue(is_form_2("1,2,3", dialect))
-        self.assertTrue(is_form_2("1,2,3\na,b,c\n", dialect))
-        self.assertTrue(is_form_2("[email protected],3", dialect))
-        self.assertTrue(is_form_2("a,,3\n1,2,3", dialect))
+        self.assertTrue(is_form_2("1,2,3".split("\n"), dialect))
+        self.assertTrue(is_form_2("1,2,3\na,b,c".split("\n"), dialect))
+        self.assertTrue(is_form_2("[email protected],3".split("\n"), dialect))
+        self.assertTrue(is_form_2("a,,3\n1,2,3".split("\n"), dialect))
 
-        self.assertFalse(is_form_2("1,2,3\n1,2\n4,5,6", dialect))
-        self.assertFalse(is_form_2("1", dialect))
-        self.assertFalse(is_form_2('1,"a"', dialect))
-        self.assertFalse(is_form_2("a;b,3", dialect))
-        self.assertFalse(is_form_2('"a,3,3\n1,2,3', dialect))
-        self.assertFalse(is_form_2('a,"",3\n1,2,3', dialect))
+        self.assertFalse(is_form_2("1,2,3\n1,2\n4,5,6".split("\n"), dialect))
+        self.assertFalse(is_form_2("1".split("\n"), dialect))
+        self.assertFalse(is_form_2('1,"a"'.split("\n"), dialect))
+        self.assertFalse(is_form_2("a;b,3".split("\n"), dialect))
+        self.assertFalse(is_form_2('"a,3,3\n1,2,3'.split("\n"), dialect))
+        self.assertFalse(is_form_2('a,"",3\n1,2,3'.split("\n"), dialect))
 
     def test_form_3(self) -> None:
         A = SimpleDialect(delimiter=",", quotechar="'", escapechar="")
         Q = SimpleDialect(delimiter=",", quotechar='"', escapechar="")
 
-        self.assertTrue(is_form_3('A,B\nC,"D"', Q))
-        self.assertTrue(is_form_3('A,B\nC,"d,e"', Q))
+        self.assertTrue(is_form_3('A,B\nC,"D"'.split("\n"), Q))
+        self.assertTrue(is_form_3('A,B\nC,"d,e"'.split("\n"), Q))
 
-        self.assertFalse(is_form_3('A,\nC,"d,e"', Q))
-        self.assertFalse(is_form_3("3;4,B\nC,D", Q))
+        self.assertFalse(is_form_3('A,\nC,"d,e"'.split("\n"), Q))
+        self.assertFalse(is_form_3("3;4,B\nC,D".split("\n"), Q))
 
-        self.assertFalse(is_form_3('A,B\n"C",D\n', A))
-        self.assertTrue(is_form_3('A,B\n"C",D\n', Q))
+        self.assertFalse(is_form_3('A,B\n"C",D'.split("\n"), A))
+        self.assertTrue(is_form_3('A,B\n"C",D'.split("\n"), Q))
 
     def test_form_4(self) -> None:
         quoted = SimpleDialect(delimiter="", quotechar='"', escapechar="")
         unquoted = SimpleDialect(delimiter="", quotechar="", escapechar="")
 
-        self.assertTrue(is_form_4("A\nB\nC", unquoted))
-        self.assertTrue(is_form_4("1\n2\n3", unquoted))
-        self.assertTrue(is_form_4("A_B\n1\n2", unquoted))
-        self.assertTrue(is_form_4("A&B\n1\n2", unquoted))
-        self.assertTrue(is_form_4("A&B\n-1\n2", unquoted))
-        self.assertTrue(is_form_4('"A"\n"B"\n"C"\n', quoted))
+        self.assertTrue(is_form_4("A\nB\nC".split("\n"), unquoted))
+        self.assertTrue(is_form_4("1\n2\n3".split("\n"), unquoted))
+        self.assertTrue(is_form_4("A_B\n1\n2".split("\n"), unquoted))
+        self.assertTrue(is_form_4("A&B\n1\n2".split("\n"), unquoted))
+        self.assertTrue(is_form_4("A&B\n-1\n2".split("\n"), unquoted))
+        self.assertTrue(is_form_4('"A"\n"B"\n"C"'.split("\n"), quoted))
 
-        self.assertFalse(is_form_4('"A", "B"\n"B"\n"C"\n', quoted))
-        self.assertFalse(is_form_4('"A","B"\n"B"\n"C"\n', quoted))
-        self.assertFalse(is_form_4('"A@b"\n"B"\n"C"\n', quoted))
-        self.assertFalse(is_form_4('A\n"-1"\n2', unquoted))
-        self.assertFalse(is_form_4("A B\n-1 3\n2 4", unquoted))
+        self.assertFalse(is_form_4('"A", "B"\n"B"\n"C"'.split("\n"), quoted))
+        self.assertFalse(is_form_4('"A","B"\n"B"\n"C"'.split("\n"), quoted))
+        self.assertFalse(is_form_4('"A@b"\n"B"\n"C"'.split("\n"), quoted))
+        self.assertFalse(is_form_4('A\n"-1"\n2'.split("\n"), unquoted))
+        self.assertFalse(is_form_4("A B\n-1 3\n2 4".split("\n"), unquoted))
 
     def test_form_5(self) -> None:
         dialect = SimpleDialect(delimiter=",", quotechar='"', escapechar="")
 
-        self.assertTrue(is_form_5('"A,B"\n"1,2"\n"3,4"', dialect))
-        self.assertTrue(is_form_5('"A,B"\n"1,"\n"2,3"', dialect))
+        self.assertTrue(is_form_5('"A,B"\n"1,2"\n"3,4"'.split("\n"), dialect))
+        self.assertTrue(is_form_5('"A,B"\n"1,"\n"2,3"'.split("\n"), dialect))
 
-        self.assertFalse(is_form_5("A,B\n1,2\n3,4", dialect))
-        self.assertFalse(is_form_5("A,B\n1,\n2,3", dialect))
-        self.assertFalse(is_form_5('"A,""B"""\n"1,"\n"2,3"', dialect))
+        self.assertFalse(is_form_5("A,B\n1,2\n3,4".split("\n"), dialect))
+        self.assertFalse(is_form_5("A,B\n1,\n2,3".split("\n"), dialect))
+        self.assertFalse(
+            is_form_5('"A,""B"""\n"1,"\n"2,3"'.split("\n"), dialect)
+        )
 
 
 if __name__ == "__main__":