astanin / python-tabulate

Pretty-print tabular data in Python, a library and a command-line utility. Repository migrated from bitbucket.org/astanin/python-tabulate.
https://pypi.org/project/tabulate/
MIT License
2.18k stars 164 forks source link

Strings occasionally tried to be coerced to numbers and raise #206

Open dsoprea opened 2 years ago

dsoprea commented 2 years ago

I'll occasionally see production reporting processes fail because I'll have a floatfmt that specifies a None for a certain column but Tabulate wants to convert a value for that column, such as "42924e28", to an int and then format it using the None formatting string. To use a particular script as an example, this value is actually the first part of a UUID. If I tell the script to display the whole UUID (e.g. "42924e28-69c5-47f9-be11-39279a9dd8ee", in this example), the table will render fine.

This is the exception:

...
    print(tabulate.tabulate(lines, headers=headers, floatfmt=floatfmt))
  File "/home/ubuntu/.virtualenvs/workflow_application/lib/python3.8/site-packages/tabulate.py", line 1591, in tabulate
    cols = [
  File "/home/ubuntu/.virtualenvs/workflow_application/lib/python3.8/site-packages/tabulate.py", line 1592, in <listcomp>
    [_format(v, ct, fl_fmt, miss_v, has_invisible) for v in c]
  File "/home/ubuntu/.virtualenvs/workflow_application/lib/python3.8/site-packages/tabulate.py", line 1592, in <listcomp>
    [_format(v, ct, fl_fmt, miss_v, has_invisible) for v in c]
  File "/home/ubuntu/.virtualenvs/workflow_application/lib/python3.8/site-packages/tabulate.py", line 996, in _format
    return format(float(val), floatfmt)
TypeError: format() argument 2 must be str, not None

For now, I've had to disable the float formatting. It makes this tool unstable and disruptive.

astanin commented 2 years ago

You can use disable_numparse option to avoid converting strings to numbers:

>>> print(tabulate([["42924e28"]]))
----------
4.2924e+32
----------
>>> print(tabulate([["42924e28"]], disable_numparse=True))
--------
42924e28
--------
dsoprea commented 2 years ago

This is not hugely convenient. It can be a fairly systemic issue used in dozens or hundreds of places before the first time it comes up. I don't know if the same can be said of the opposite behavior. Only a very small fraction of people may desire to have this enabled by default. Maybe we can consider disabling it by default in the next major release? Maybe we can take a vote (leave an issue open and advertise it in bold in the README)?

astanin commented 2 years ago

Converting strings to numbers is the library feature from the day one (because that's what was convenient for me when I wrote the library). It's documented in the README https://github.com/astanin/python-tabulate/blob/master/README.md?plain=1#L640-L665 and at this point it's likely to stay this way forever even if for backwards compatibility alone.

I'll reopen the issue to gather further feedback and ideas, but this preference is not likely to be changed.

Voting is meaningless because most of the users will never open this thread. But their code should still work if possible. I believe that numbers formatted as strings are a more common use case, than hex numbers (hashes, UUIDs, etc). And users printing UUIDs and hex numbers are also more likely to be tech savvy to activate the option.

Regarding the choice of the default parameters I may often agree that some choices could have been more convenient, but it's almost impossible to measure how often each feature is used and by how many users. At this point changing these preferences may break workflows and code of other people.

For users who have different preferences I recommend creating a wrapper around the library function. Something like this:

>>> def tabulate_dontparsenums(*args, **kwargs):
...     new_defaults = {"disable_numparse": True}
...     new_defaults.update(kwargs)
...     return tabulate(*args, **new_defaults)
...
>>> print(tabulate_dontparsenums([["42924e28"]], tablefmt="grid"))
+----------+
| 42924e28 |
+----------+
dsoprea commented 2 years ago

I appreciate backwards compatibility. That's why I figured we could keep this on a dusty shelve until the next major release.

Thanks, @astanin.