Fix ODS export of boolean values crashing on re-import

[vineethsaivs]

What

Exporting a Dataset that contains a boolean to ODS and reading it back crashes:

import tablib
d = tablib.Dataset()
d.headers = ['name', 'flag']
d.append(['alice', True])
d.append(['bob', False])
tablib.Dataset().load(d.export('ods'), 'ods')
# ValueError: could not convert string to float: 'True'

In Python bool is a subclass of int, so a True/False cell satisfies isinstance(col, numbers.Number) in ODSFormat.dset_sheet and is written with valuetype="float" and value=True. odfpy serializes that attribute as the string "True", and on import read_cell runs float("True") for a float-typed cell, raising ValueError.

read_cell already has a value_type == 'boolean' reader, but the export side never produced a boolean-typed cell, so that path was unreachable for tablib's own output.

Fix

Add a bool branch before the numbers.Number branch in dset_sheet that writes valuetype="boolean" with the matching booleanvalue, so booleans round-trip back as Python bool. The bool check must come first because bool is a subclass of int.

Test

Added ODSTests.test_ods_export_import_boolean, which round-trips True and False through ODS and asserts they come back as the correct booleans. It fails before the change (ValueError) and passes after. The existing numeric/date round-trip test is unaffected.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.24%. Comparing base (bfecdaa) to head (738bdf7).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #656      +/-   ##
==========================================
+ Coverage   93.22%   93.24%   +0.02%     
==========================================
  Files          29       29              
  Lines        3261     3272      +11     
==========================================
+ Hits         3040     3051      +11     
  Misses        221      221              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[claudep]

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]

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]

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.

[vineethsaivs]

Good point, thanks. The assertion should verify the values come back as real booleans, not merely truthy/falsy (that is the whole point of the fix, since before it they round-tripped as the string 'True'). I have updated the test to use hugovk's variable-based form:

for index, expected in ((0, True), (1, False)):
    self.assertIs(data.dict[index]['flag'], expected)

This keeps the strict assertIs check and, since teyit only matches bare True/False AST constants, it leaves this alone, so no change to the teyit/pre-commit config is needed. I will leave the unittest-vs-pytest question to you.

[claudep]

Thanks, but since @hugovk removed the teyit linter, I think you could revert to your original version, sorry!

[vineethsaivs]

Done, reverted to the direct assertIs(data.dict[0]['flag'], True) / assertIs(data.dict[1]['flag'], False) form now that teyit is gone. Thanks!

[claudep]

Perfect, thanks!