fixed _isbool for python 3 #62
Conversation
tabulate.py
Outdated
| @@ -610,13 +610,21 @@ def _isbool(string): | |||
| """ | |||
| >>> _isbool(True) | |||
| True | |||
| >>> _isbool("False") | |||
| >>> _isbool(b"false") | |||
There was a problem hiding this comment.
Shouldn't it be b"False" rather than b"false"?
I'd rather see a different test case for b"False" and "False", than the old test being replaced with a new test.
There was a problem hiding this comment.
I think it should accept both b'false' and b'False', to handle data that doesn't originate from python. For example, in mycli we sometimes get booleans as strings from the SQL connector and would like to display them without parsing. But maybe that's asking too much.
There was a problem hiding this comment.
I understand where it's coming from, but it's a slippery slope. In no time we'll have to maintain all possible ways to format false and true values (.TRUE., FaLsE, nil, ...). Probably as a compromise we may decide to accept only "true" and "false", but let them be case-insensitive.
The argument for supporting only "True" and "False": these are Python literals, this is how the library already works.
The argument to supporting "True", "true", "False", and "false": it makes it easier to consume output generated by other programming languages. The argument against: it's a breaking change. The behavior of _isbool("false") -> False was not documented, but this PR will change it.
The argument to do a case-insensitive match: the same as above.
I'm very reluctant to do breaking changes to this library. Its heuristics are sort of odd, and at this point I'm pretty sure there's someone who relies on "false" being literal text. But I think your suggestion is more practical.
_isbool works incorrectly with binary data in Python 3: