Skip to content
This repository has been archived by the owner on May 26, 2022. It is now read-only.

XLSX Date Writer #816

Closed
wants to merge 3 commits into from
Closed

XLSX Date Writer #816

wants to merge 3 commits into from

Conversation

rodnaph
Copy link

@rodnaph rodnaph commented May 24, 2021

Adds support for writing instances of DateTimeInterface (ie. DateTime and DateTimeImmutable) with the XLSX writer.

Previous issues such as #781 and #797 have been closed with regards this issue, but I believe #781 is actually valid. From my research (disclaimer: I'm new to this XML format...) it seems the numeric date format for "serial dates" referenced in the closing comment of #781 are those originally supported by openxml, for example...

<c s="14">
  <v>44401</v>
</c>

However, ISO/IEC 29500-1:2016 adds support for ISO dates using the type d.

https://www.iso.org/standard/71691.html

image

<c t="d" s="14">
  <v>2021-02-03T12:34:45</v>
</c>

Thoughts? If this is valid I'm happy to re-work this PR as needed, cheers.

This methods was untested, so this adds basic support for all data types
currently tested by other test cases in this class.
Adds support for DateTimeImmutable by using the shared DateTimeInterface
which both this and DateTime implement.
This adds support for writing DateTime and DateTimeImmutable values with
the XLSX writer. The dates are written using the date type (t="d") which
must then specify an ISO date as the value (as opposed to a linear date
in previous versions).
@adrilo
Copy link
Collaborator

adrilo commented May 28, 2021

Hi @rodnaph,

While your solution is technically true, I did a quick test this in different softwares and none of them supported this format:

  • Numbers on Mac: date appears as a number
  • Google Sheets: values were simply discarded (blank cells)

This is unfortunately a deal breaker...

@adrilo adrilo closed this May 28, 2021
@rodnaph
Copy link
Author

rodnaph commented May 28, 2021

I tested Numbers on Mac 6.2 and it does support this format, I'm surprised Google Sheets doesn't though... I'll investigate and re-open this if I can find a way around it.

@rodnaph
Copy link
Author

rodnaph commented May 29, 2021

Would you accept a PR that omitted the t="d" attribute and instead wrote the DateTimeInterface as a serial date? (eg. 40021)

@adrilo
Copy link
Collaborator

adrilo commented Jun 1, 2021

Using a timestamp would be the best approach indeed. But it requires setting a cell style (there are some built-in styles for dates that can be used). This also poses some questions:

  • should you let the user choose the date format?
  • If not, which format should you choose? Is this format localized?

@Slamdunk
Copy link

Slamdunk commented Mar 3, 2022

Solved in openspout/openspout#13

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants