Convert XLS cell values when importing a Databook - #665
Conversation
XLSFormat.import_book re-implemented the row loop with raw sheet.row_values(), so the same file loaded as a Databook returned Excel date serials and raw error codes where a Dataset returned datetime objects and strings like #N/A. Extract an import_sheet classmethod, mirroring the XLSX and ODS formats, and call it from both import_set and import_book so the two paths cannot drift again. The cell conversion moves into a read_cell classmethod, matching the ODS format, with the workbook datemode passed in explicitly.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #665 +/- ##
==========================================
+ Coverage 93.42% 93.47% +0.05%
==========================================
Files 29 29
Lines 3298 3308 +10
==========================================
+ Hits 3081 3092 +11
+ Misses 217 216 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
The only red check here is Recap: |
claudep
left a comment
There was a problem hiding this comment.
Looks good, thanks for this fix!
The bug
XLSFormat.import_setreads cells through acell_valuehelper that turns Excel date serials intodatetimeobjects and error codes into strings like#N/A.XLSFormat.import_bookre-implemented the row loop with rawsheet.row_values(i)and never got that helper, so the same file returns different values depending on whether you load it as aDatasetor aDatabook.Using the fixtures already in the test suite:
That
42is the symptom from #202 ("Tablib import Excel NA() values as 42"), which was fixed on theimport_setpath only; it still reproduces today throughDatabook.XLS is the only spreadsheet format where the two paths differ. Both XLSX and ODS delegate
import_setandimport_bookto a sharedimport_sheet, so they cannot drift. The existingtest_xls_date_importandtest_xls_import_with_errorsassert the converted values, but only viaDataset, and the one XLS Databook test only checks a sheet title, which is why this went unnoticed.The fix
Extract an
import_sheetclassmethod and call it from both entry points, mirroring the structure_xlsx.pyand_ods.pyalready use. The cell conversion becomes aread_cellclassmethod, matching theread_cellin_ods.py, with the workbookdatemodepassed in explicitly rather than captured from an enclosing scope (sheet.bookis not reliable under xlrd'son_demand).Header handling is deliberately unchanged.
import_setreads headers with rawrow_valuestoday and the shared helper keeps doing that, soDatabooknow matchesDatasetexactly rather than changing both.Tests
Two regression tests load the existing
dates.xlsanderrors.xlsfixtures as aDatabookand assert the same values theDatasettests already assert. Both fail before this change (42106.0 != datetime.datetime(2015, 4, 12, 0, 0)) and pass after.Full suite passes before and after; the count goes from 180 to 182 with the two new tests. Added myself to
AUTHORSper the contributing guide.