Fixes #314 - Delegate type coercion to openpyxl #386
Conversation
test_tablib.py
Outdated
|
|
||
| def test_xlsx_wrong_char(self): | ||
| """A bad character should not crash the export""" | ||
| data.append(('string', b'\x0cf')) |
There was a problem hiding this comment.
@hugovk That test currently fails and I'd like to know your opinion. openpyxl is refusing to store some chars in a certain range:
https://bitbucket.org/openpyxl/openpyxl/src/15df72eae2f93bedc0c0c0cc8753b9107cc82d41/openpyxl/cell/cell.py#lines-70
Should we let the error break the export, or catch it and replace the offended chars by some value (empty string, '#N/A'?) I tend to favor letting the exception crash the export (and change the test accordingly).
There was a problem hiding this comment.
Right now it's raising IllegalCharacterError:
Traceback (most recent call last):
File "test_tablib.py", line 798, in test_xlsx_wrong_char
data.xlsx
File "/home/travis/build/jazzband/tablib/tablib/formats/_xlsx.py", line 39, in export_set
dset_sheet(dataset, ws, freeze_panes=freeze_panes)
File "/home/travis/build/jazzband/tablib/tablib/formats/_xlsx.py", line 142, in dset_sheet
cell.value = unicode(col, errors='ignore')
File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/openpyxl/cell/cell.py", line 239, in value
self._bind_value(value)
File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/openpyxl/cell/cell.py", line 215, in _bind_value
value = self.check_string(value)
File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/openpyxl/cell/cell.py", line 182, in check_string
raise IllegalCharacterError
openpyxl.utils.exceptions.IllegalCharacterError
I think that's probably the right thing to do, it will let people know what the problem is and let them react appropriately (change the character, delete it, etc.).
@cristiano2lopes, what do you think?
Thanks Cristiano Lopes for the initial patch.
|
I think I will push the patch in the current state, and we can continue discussing the illegal characters exception in #370. |
Thanks Cristiano Lopes for the initial patch.