is_date is wrong for readonly cells

Issue #1174 resolved
Scott Stanton created an issue

The implementations for Cell.is_date and ReadOnlyCell.is_date are different. ReadOnlyCell does not handle self.data_type=='d'.

Comments (6)

  1. CharlieC

    Thanks, must have missed this. In 2.6 both implementations finally share the same parser where the type casting happens. As a workaround you can now cheaply test the value with is_instance()

  2. CharlieC

    While I look at fixing this, I'm wondering what the correct behaviour is. Historically, we've followed Excel's convention of relying on the number format, but we switched to data typing a while back. For example, how should we handle a cell that has no value but has the number format for a date? Currently is_date will tell you it's a date but I think this is probably a case of 3-valued logic, because None clearly isn't a date or a datetime.

    The more I think about it, the more it seems that client code should really decide. The cell's datatype is probably sufficient for most cases, and otherwise it's better that client code explicitly checks the number_format. In this case it's probably time to mark the property as deprecated.

  3. Scott Stanton reporter

    I think returning true when the type value indicates it's a date probably makes sense. Testing for the date format is probably more speculative and, as you say, is ambiguous for nulls. To my mind, hiding the type constants is the main requirement for the api. The client code shouldn't need to know about the magic value 'd'. I'd be ok with it only returning true for type matches, or at least adding an enum to define the legal type constants.

  4. CharlieC

    I think it's debatable as to whether the API promotes shorthand OOXML types or properties. We could probably adjust the API to return a friendlier data_type property. Discussion should probably continue on the mailing list.

    In the meantime, I've fixed the property so that the property is the same for both modes.

  5. Log in to comment