Skip to content

Fix ODS export of boolean values crashing on re-import #656

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
claudep merged 5 commits into from
Jul 15, 2026
+16 −1
Merged

Fix ODS export of boolean values crashing on re-import #656

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aea4bd7
Fix ODS export of boolean values crashing on re-import
vineethsaivs Jul 12, 2026
bb9326c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 12, 2026
e53e83e
Test ODS boolean round-trip with assertIs to verify real booleans
vineethsaivs Jul 13, 2026
738bdf7
Wrap boolean ODS cell construction to satisfy line-length lint
vineethsaivs Jul 13, 2026
f493f31
Assert booleans directly now that teyit is removed
vineethsaivs Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/tablib/formats/_ods.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,9 @@ def dset_sheet(cls, dataset, ws):
odf_row = table.TableRow(stylename=style)
ws.addElement(odf_row)
for j, col in enumerate(row):
if isinstance(col, numbers.Number):
if isinstance(col, bool):
cell = table.TableCell(valuetype="boolean", booleanvalue="true" if col else "false")
elif isinstance(col, numbers.Number):
cell = table.TableCell(valuetype="float", value=col)
elif isinstance(col, dt.datetime):
cell = table.TableCell(
Expand Down
10 changes: 10 additions & 0 deletions tests/test_tablib.py
100755 → 100644
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,16 @@ def test_ods_export_import_set(self):
self.assertEqual(data.dict[0]['None'], '')
self.assertEqual(data.dict[0]['empty'], '')

def test_ods_export_import_boolean(self):
data.append(('alice', True))
data.append(('bob', False))
data.headers = ('name', 'flag')
_ods = data.ods
data.ods = _ods
self.assertTrue(data.dict[0]['flag'])
self.assertFalse(data.dict[1]['flag'])

@claudep claudep Jul 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not agree with this precommit change, we clearly want to test False/True here, and not the truethy/falsy values. @hugovk, do you know if we can by-pass the automatic change in this particular case?

@hugovk hugovk Jul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is from teyit, it doesn't have # noqa way to disable per line.

Some options:

  • Flip the order: self.assertIs(True, data.dict[0]['flag'])

  • Rewrite as two checks: assertIsInstance(flag, bool) and assertTrue(flag)

  • It only matches AST constants, use variables:

  for index, expected in ((0, True), (1, False)):
      self.assertIs(data.dict[index]['flag'], expected)
  • Exclude teyit for this file in .pre-commit-config.yaml (which has the bulk of the tests)
  • Disable teyit entirely

Somewhat related, I use pytest in most other projects. What do you think about using it here, or do you prefer unittest? I'd prefer pytest, but also fine with unittest too.

@claudep claudep Jul 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of automated checks, so I wouldn't be opposed to simply disable teyit entirely.

As far as testing is concerned, I've never been convicted about the real added value of pytest. For me it's more a matter of preferences, so when I can stick to the standard lib, I do. But I don't consider this as my project, so feel free to change that if you have the time for.

self.assertEqual(data.dict[0]['name'], 'alice')

def test_ods_export_display(self):
"""Test that exported datetime types are displayed correctly in office software"""
date = dt.date(2019, 10, 4)
Expand Down
Loading