Fix issue with DateTime cells in DataSet from .xls

Apr 21, 2011 at 2:06 PM

There was a bug in ExcelBinaryReader: when read file as DataSet DateTime cells often read incorrectly.

I figured out the excel file format and fixed DateTime format detection for NUMBER_OLD, RK, MUL_RK and FORMULA* cell types, which may contain the date.

I would be glad if you could review and test my patch.

Please, comment if i did something wrong, this is my first participation in open source.

Apr 25, 2011 at 11:44 PM
Edited Apr 26, 2011 at 8:30 PM

Dear Ruddenko,

Before anythink else: thanks for your time!

I'm JP Negri, the negrijp from the developers list.

I downloaded and applied your patch to my local copy to revise. It all worked fine, excepet a unit test on ExcelBinaryReaderTest.Test_num_double_date_bool_string() line 353. The retrieved value misses the time part, as expected in the test.

Is realy the intention retrieve only what is displayed on screen (in my case "2009-05-07") or the underlying value (2009-06-07 11:01:02)? If the answer is the first, the we must change the unitary test to reflect that.

It´s my personal view and usage that the library should not try to convert numbers to dates, as Excel itself does not have an internal representation for dates, and trying this based on formating is trick. But, as this is an Open Source project, we must listem to the peoples choice, and just deliver.

What is your opinion? Anyone want to talk about this?

I will hold on a few days before changing the unity test and uploading a new revision to the trunk.

My best regards,

JP Negri

Apr 27, 2011 at 1:04 AM

I also uploaded a patch for this and other type conversion issues (preserve data types patch).  My feeling is that the internal representation of dates is irrelevant.  Functionally, excel does have the concept of a date, boolean, etc.  If you look at alternatives, such as the microsoft ado driver as well as other commercial products, these datatypes are not lost.  I imagine that for most functional purposes, maintaining datatypes would be desirable.  With regard to the AsDataSet method, it can specify to convert OA dates, yet it doesn't always do so.  Perhaps this could be configurable for all datatypes??

Apr 27, 2011 at 11:50 AM

Hello, JP Negri!

I consider that we have to convert OA date from it binary representation (number), if the corresponding cell format indicated that this is date. But if anyone need - it can be customisable to turn off this convertion.

About the fact that we rely on the cell type to determine that it stores the date - it's not a trick it corresponds with specification.

No need to changing unit test, because it have to read entire date with time part as it written to cell F1 ([0,5]) in test file (I see that as "07.05.2009  11:01:02"). On my computer, test work correctly, perhaps it can be caused by Culture settings. I think it would be better if I retest it with your System Culture and maybe fix, before you uploading to trunk. 

What Culture you use in your system?

Your sincerely, Andrey.