Repository: astanin/python-tabulate

Issue:
PRESERVE_WHITESPACE should be made an argument
It's currently a module-level global variable. This is assuming that all users of tabulate uses the same PRESERVE_WHITESPACE setting. This will not work well if different use of tabulate needs different settings.

Having an argument for this setting would be more general.

Discussion:
astanin: This is a breaking change, so it will be postponed at least until the next major release.

What is the reason to remove PRESERVE_WHITESPACE macro? I'm pretty sure some users rely on it.
vrza: The reasons are cleanliness of design and clear user facing API, with no [monkey patching](https://en.wikipedia.org/wiki/Monkey_patch) necessary.

To explain in detail, `PRESERVE_WHITESPACE` is not a macro, but rather a constant defined as a property of the `tabulate` module. In order to change the whitespace behavior this constant affects, the consumer must monkey patch the tabulate module. You can see an example of this in [this unit test](https://github.com/astanin/python-tabulate/blob/bfc922cb04aa4a8bfe6360a50ac774d513605043/test/test_output.py#L1601).

Monkey patching alters the default module state and this can lead to problems. For example, if one user function monkey patches the module at runtime, and doesn't patch it back to its original state after use, a second user function, called subsequently, that's relying on the default behavior, might fail or produce unexpected results.

You can see this in action by splitting the `test_preserve_whitespace` unit test into two tests (and it really does contain two distinct tests, hiding inside one function):

```
def test_preserve_whitespace_true():
    "Output: Default table output, but with preserved leading whitespace."
    tabulate_module.PRESERVE_WHITESPACE = True
    table_headers = ["h1", "h2", "h3"]
    test_table = [["  foo", " bar   ", "foo"]]
    expected = "\n".join(
        ["h1     h2       h3", "-----  -------  ----", "  foo   bar     foo"]
    )
    result = tabulate(test_table, table_headers)
    assert_equal(expected, result)

def test_preserve_whitespace_default():
    table_headers = ["h1", "h2", "h3"]
    test_table = [["  foo", " bar   ", "foo"]]
    expected = "\n".join(["h1    h2    h3", "----  ----  ----", "foo   bar   foo"])
    result = tabulate(test_table, table_headers)
    assert_equal(expected, result)
```
    
If `test_preserve_whitespace_default` runs first, and `test_preserve_whitespace_true` second, both tests will pass. However, if the `test_preserve_whitespace_true` runs first, then `test_preserve_whitespace_default` will fail, because the first test monkey patches the tabulate module and doesn't patch it back after use.

You can read about more monkey patching pitfalls in [the wikipedia article.](https://en.wikipedia.org/wiki/Monkey_patch). For these reasons, it's not a good idea to make monkey patching a standard way of using the library. This is why I moved the `preserve_whitespace` tunable to the constructor, as an optional argument with a default value, resulting in a cleaner API which is less error-prone to use. The functionality is still there, it is just exposed to the user in a more user-friendly style.

Indeed, this change will break user programs that rely on monkey patching the `tabulate.PRESERVE_WHITESPACE` property, and as such, according to semantic versioning guidelines, seems to warrant a major version bump.
astanin: Related: #37 
astanin: I generally agree with you. I'm going to integrate your changes in the 0.9 release, whenever it can be.

Patch:
diff --git a/tabulate.py b/tabulate.py
index 55b896e..541cf29 100644
--- a/tabulate.py
+++ b/tabulate.py
@@ -68,9 +68,6 @@ __version__ = "0.8.10"
 # minimum extra space in headers
 MIN_PADDING = 2
 
-# Whether or not to preserve leading/trailing whitespace in data.
-PRESERVE_WHITESPACE = False
-
 _DEFAULT_FLOATFMT = "g"
 _DEFAULT_MISSINGVAL = ""
 # default align will be overwritten by "left", "center" or "decimal"
@@ -810,13 +807,13 @@ def _choose_width_fn(has_invisible, enable_widechars, is_multiline):
     return width_fn
 
 
-def _align_column_choose_padfn(strings, alignment, has_invisible):
+def _align_column_choose_padfn(strings, alignment, has_invisible, preserve_whitespace):
     if alignment == "right":
-        if not PRESERVE_WHITESPACE:
+        if not preserve_whitespace:
             strings = [s.strip() for s in strings]
         padfn = _padleft
     elif alignment == "center":
-        if not PRESERVE_WHITESPACE:
+        if not preserve_whitespace:
             strings = [s.strip() for s in strings]
         padfn = _padboth
     elif alignment == "decimal":
@@ -830,7 +827,7 @@ def _align_column_choose_padfn(strings, alignment, has_invisible):
     elif not alignment:
         padfn = _padnone
     else:
-        if not PRESERVE_WHITESPACE:
+        if not preserve_whitespace:
             strings = [s.strip() for s in strings]
         padfn = _padright
     return strings, padfn
@@ -873,9 +870,10 @@ def _align_column(
     has_invisible=True,
     enable_widechars=False,
     is_multiline=False,
+    preserve_whitespace=False
 ):
     """[string] -> [padded_string]"""
-    strings, padfn = _align_column_choose_padfn(strings, alignment, has_invisible)
+    strings, padfn = _align_column_choose_padfn(strings, alignment, has_invisible, preserve_whitespace)
     width_fn = _align_column_choose_width_fn(
         has_invisible, enable_widechars, is_multiline
     )
@@ -1224,6 +1222,7 @@ def tabulate(
     showindex="default",
     disable_numparse=False,
     colalign=None,
+    preserve_whitespace=False
 ):
     """Format a fixed width table for pretty printing.
 
@@ -1603,7 +1602,7 @@ def tabulate(
         [width_fn(h) + min_padding for h in headers] if headers else [0] * len(cols)
     )
     cols = [
-        _align_column(c, a, minw, has_invisible, enable_widechars, is_multiline)
+        _align_column(c, a, minw, has_invisible, enable_widechars, is_multiline, preserve_whitespace)
         for c, a, minw in zip(cols, aligns, minwidths)
     ]
 


Test patch:
diff --git a/test/test_output.py b/test/test_output.py
index abab439..660f047 100644
--- a/test/test_output.py
+++ b/test/test_output.py
@@ -1598,18 +1598,16 @@ def test_disable_numparse_list():
 
 def test_preserve_whitespace():
     "Output: Default table output, but with preserved leading whitespace."
-    tabulate_module.PRESERVE_WHITESPACE = True
     table_headers = ["h1", "h2", "h3"]
     test_table = [["  foo", " bar   ", "foo"]]
     expected = "\n".join(
         ["h1     h2       h3", "-----  -------  ----", "  foo   bar     foo"]
     )
-    result = tabulate(test_table, table_headers)
+    result = tabulate(test_table, table_headers, preserve_whitespace=True)
     assert_equal(expected, result)
 
-    tabulate_module.PRESERVE_WHITESPACE = False
     table_headers = ["h1", "h2", "h3"]
     test_table = [["  foo", " bar   ", "foo"]]
     expected = "\n".join(["h1    h2    h3", "----  ----  ----", "foo   bar   foo"])
-    result = tabulate(test_table, table_headers)
+    result = tabulate(test_table, table_headers, preserve_whitespace=False)
     assert_equal(expected, result)