Skip to content

Accept ISO-8601 short time offset format#5015

Merged
archmoj merged 1 commit into
plotly:masterfrom
mtgto:iso8601_short_tz
Jul 24, 2020
Merged

Accept ISO-8601 short time offset format#5015
archmoj merged 1 commit into
plotly:masterfrom
mtgto:iso8601_short_tz

Conversation

@mtgto

@mtgto mtgto commented Jul 23, 2020

Copy link
Copy Markdown
Contributor

#1194 supports ISO8601 timestamp format.
lib/dates.js says it supports the ISO8601 time zone format.
In fact it supports the RFC3339 time offset format, not ISO8601.

time offset format of RFC3339 is:

time-numoffset  = ("+" / "-") time-hour ":" time-minute

https://tools.ietf.org/html/rfc3339#section-5.6

It always needs minutes.

In contract, ISO8601 time offset format can abbreviate minute if minute is 00.

The UTC offset is appended to the time in the same way that 'Z' was above,
in the form ±[hh]:[mm], ±[hh][mm], or ±[hh].

https://en.wikipedia.org/wiki/ISO_8601#Time_offsets_from_UTC


I noticed this problem while converting PostgreSQL response into chart.
Plotly.js does not allow the date and time output from PostgreSQL.

Example of datetime string from PostgreSQL: 1997-12-17 07:37:16-08 (minute of time offset is abbreviated)

The output format of the date/time types can be set to one of the four styles ISO 8601, SQL (Ingres), traditional POSTGRES (Unix date format), or German. The default is the ISO format.

https://www.postgresql.org/docs/12/datatype-datetime.html#DATATYPE-DATETIME-OUTPUT

@archmoj archmoj added status: reviewable community community contribution labels Jul 23, 2020

@alexcjohnson alexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃 Thanks @mtgto - this looks like a useful addition, the tests are perfect. @archmoj unless you have any other comments feel free to merge when you're ready.

@archmoj archmoj added the bug something broken label Jul 24, 2020
@archmoj
archmoj merged commit 556b5d4 into plotly:master Jul 24, 2020
@mtgto

mtgto commented Jul 24, 2020

Copy link
Copy Markdown
Contributor Author

Thanks for your review 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug something broken community community contribution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants