Datetime data type #364

alexcjohnson · 2019-02-07T04:09:22Z

Added a 'datetime' data type. This type accepts strings in exactly the same forms as plotly.js accepts, which can be dates or date-times, in the form 'YYYY-mm-dd HH:MM:SS.ssssss' truncated anywhere and with a few extras allowed for flexibility. I chose to call the type 'datetime' rather than simply 'date' on the assumption that we will at some point also want to add a 'date' type (that does not accept times, only the date portion) and a 'time' type (that has no date, only the time).

For now the string gets no special formatting, but leading and trailing spaces are removed.

alexcjohnson · 2019-02-07T04:19:50Z

FYI package-lock.json changed without a package.json change because I ran npm i using npm@6.4, presumably it was generated by npm@5.x before. Any objection to using 6.x going forward?

alexcjohnson · 2019-02-07T14:50:05Z

tests/cypress/src/DashTable.ts

+    static focusCell(row: number, column: number) {
+        // somehow we need to scrollIntoView AFTER click, or it doesn't
+        // work right. Why?
+        return this.getCell(row, column).click().scrollIntoView();


You'd think we should first scroll the cell into view, and THEN click it... but that doesn't solve the problem (that on my machine we can't type into the cell because Cypress thinks it's hidden), it's only solved by scrolling after the click. I gave up trying to figure out why...

I find this weird. Never had to scroll into view before. Normally Cypress usually scrolls the element into view automatically -- it's disabled for certain tests by default (e.g. with virtualization) as it caused issues.

alexcjohnson · 2019-02-07T14:57:41Z

CHANGELOG.md

@@ -3,6 +3,10 @@ All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).

 ## [Unreleased]
+### Added
+[#364](https://github.com/plotly/dash-table/pull/224)
+- Added the `datetime` data type


@Marc-Andre-Rivet I notice a lot of the table changelog refers to issues. Not sure if it matters to users, but to me as a dev it's a lot more useful to refer to PRs, so you can go straight from the changelog to the code that changed.

One downside, of course, is that this commit has to be added after the PR is opened and you know its number (at least as long as we keep updating the changelog in the PR, which I still find debatable 😅 )

My thinking was as you said, that having to create the PR in order to re-modify the changelog was pretty annoying. Also, and more importantly, the PR exposes the internal details of the modifications made to attain a certain fix/change in behavior/additional feature -- which should not be of importance to most users of our product. A well structured issue seems more natural as it could/should expose the details of the change from the perspective of a user, not a developer (or the changelog itself, or a community post). This means we should make our issues more meaningful though :)

A quick check on some major projects shows there's absolutely no standard! React links to PRs, VueJS to issues, Cypress to issues, Webpack to actual commits.

Marc-Andre-Rivet · 2019-02-07T15:14:34Z

src/dash-table/reconcile/date.ts

+// (simplified - no international calendars for now)
+// https://github.com/plotly/plotly.js/blob/master/src/lib/dates.js
+// Note we allow timezone info but ignore it - at least for now.
+const DATETIME_REGEXP = /^\s*(-?\d\d\d\d|\d\d)(-(\d?\d)(-(\d?\d)([ Tt]([01]?\d|2[0-3])(:([0-5]\d)(:([0-5]\d(\.\d+)?))?(Z|z|[+\-]\d\d:?\d\d)?)?)?)?)?\s*$/m;


Any reason for not using the terser \d{4}, \d{2}, \d{1,2}?

Same question for the 'i' flag to make it case insensitive (T|t, Z|z)

No reason, just copied over from plotly.js. I could tighten up the \d usage here, though I might prefer to keep [ Tt] and (Z|z|... over /i, seems more explicit since letters are a very small fraction of the overall pattern.

Kk, I'm fine with it as is in any case.

Marc-Andre-Rivet · 2019-02-07T15:53:28Z

CHANGELOG.md

@@ -3,6 +3,10 @@ All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).

 ## [Unreleased]
+### Added
+[#364](https://github.com/plotly/dash-table/pull/224)


fixed in 12b1597

Marc-Andre-Rivet · 2019-02-07T16:18:21Z

src/dash-table/reconcile/date.ts

+        {
+            success: true,
+            // TODO: also convert to 4-digit year and 2-digit month & day?
+            value: String(value).trim()


Here I was expecting the coerce to return an actual date or minimally a ISO formatted date instead of the original value.

What do you mean by "an actual date"?

But yes, we can definitely have coerce standardize the format. I think it should still respect the fields the user provided though - so standardize the number of digits in years/months/days, but don't add any information that wasn't already there.

What do you mean by "an actual date"?

The Date object/instance.

a4cdb11 makes the changes as discussed:

Prohibits years-as-numbers - only strings are allowed

Prohibits 2-digit years by default, unless validation.allow_YY=true. Adds suggested Python and R date parsers and a warning about trying to parse 2-digit years.

coerce normalizes the output format but preserves the precision supplied by the user.

Please take a look at how this plays out in the tests, let me know if it all looks reasonable.

Marc-Andre-Rivet · 2019-02-07T18:06:02Z

As dicussed,

adding suggested Dash app implementation for handling dates
adding opt-in for 2-digits date format + warning about the consequences

- prohibit years-as-numbers - prohibit YY by default, unless validation.allow_YY=true - coerce normalizes the output format

Marc-Andre-Rivet · 2019-02-08T15:16:08Z

src/dash-table/reconcile/date.ts

+// (simplified - no international calendars for now)
+// https://github.com/plotly/plotly.js/blob/master/src/lib/dates.js
+// Note we allow timezone info but ignore it - at least for now.
+const DATETIME_REGEXP = /^\s*(-?\d{4}|\d{2})(-(\d{1,2})(-(\d{1,2})([ Tt]([01]?\d|2[0-3])(:([0-5]\d)(:([0-5]\d(\.\d+)?))?(Z|z|[+\-]\d{2}:?\d{2})?)?)?)?)?\s*$/m;


We accept both positive and negative dates but do not accept any date in the range [0,1000[ and [-999,-1[. That said, we shouldn't accept 0 anyway :)

We do accept these, they just need leading zeros to 4 digits. eg:

input: ' 0369-11-02 15', output: '0369-11-02 15'

If allow_YY===false, maybe we should interpret all years as-is? In which case, allow_YY may not be the correct option name.

Why am I not seeing the comments until I refresh... ok for leading zeros. Maybe that should be documented?

That said, we shouldn't accept 0 either :)

In the BCE/AD counting scheme there's no year 0. But when you use negative numbers for years, there is a year zero, and it's the same as 1 BCE. -1 is 2BCE, etc...

d=new Date(); y1 = d.setFullYear(1); y0 = d.setFullYear(0); yn1 = d.setFullYear(-1); console.log((y1 - y0), (y0 - yn1)) 31622400000 31536000000 // a year is ~π*10^7 sec (*10^10 ms)

If allow_YY===false, maybe we should interpret all years as-is?

I think the main outcome of this would be to cause confusion for users who have 2-digit years, that we'd end up interpreting as the first century AD. I think the default should be to only accept 4-digit years; that's the standard in a lot of places and it's unambiguous (at least until we want to support years beyond [-9999, 9999] but we have that limit in plotly.js and AFAIK it's never come up)

I think the main outcome of this would be to cause confusion
Agreed. Now that I know I can do leading zeros!

In the BCE/AD counting scheme there's no year 0. But when you use negative numbers for years, there is a year zero, and it's the same as 1 BCE. -1 is 2BCE, etc...

Part of ISO8601.. did not know that! 💡

Marc-Andre-Rivet · 2019-02-08T15:33:23Z

src/dash-table/reconcile/date.ts

+    return normalizedDate !== null ?
+        {
+            success: true,
+            // TODO: also convert to 4-digit year and 2-digit month & day?


I think this comment does not apply anymore

removed in 68a0af3

Marc-Andre-Rivet · 2019-02-08T15:34:29Z

tests/cypress/tests/unit/dateCoercion_test.ts

+    { input: '2000/01/01', name: 'slash separated' },
+    { input: '2000 01 01', name: 'space separated' },
+    { input: '5-01-01', name: 'one-digit year' },
+    { input: '105-01-01', name: 'three-digit year' },


Here it was, the three digits year!

Marc-Andre-Rivet

💃 once the tests pass one last time!

alexcjohnson added 5 commits February 6, 2019 22:19

robustify dash_test tests so they'll run on AJ's machine

7fefccd

date data type

b60204b

dates mode in demo app

06ba23d

unit and standalone date tests

d675dc3

rebuild

066ac98

alexcjohnson added the dash-type-enhancement New feature or request label Feb 7, 2019

alexcjohnson requested a review from Marc-Andre-Rivet February 7, 2019 04:09

chriddyp temporarily deployed to dash-table-review-pr-364 February 7, 2019 04:09 Inactive

add datetime to changelog

a3cad4e

chriddyp temporarily deployed to dash-table-review-pr-364 February 7, 2019 04:12 Inactive

alexcjohnson commented Feb 7, 2019

View reviewed changes

Marc-Andre-Rivet reviewed Feb 7, 2019

View reviewed changes

fix changelog for datetime

12b1597

Marc-Andre-Rivet reviewed Feb 7, 2019

View reviewed changes

update date type system

a4cdb11

- prohibit years-as-numbers - prohibit YY by default, unless validation.allow_YY=true - coerce normalizes the output format

chriddyp temporarily deployed to dash-table-review-pr-364 February 8, 2019 03:46 Inactive

Marc-Andre-Rivet reviewed Feb 8, 2019

View reviewed changes

alexcjohnson added 2 commits February 8, 2019 11:15

update/clarify date comments

68a0af3

Merge branch 'master' into date-type

5be528e

chriddyp deployed to dash-table-review-pr-364 February 8, 2019 16:28 View deployment

Marc-Andre-Rivet approved these changes Feb 8, 2019

View reviewed changes

alexcjohnson merged commit 10396e6 into master Feb 8, 2019

alexcjohnson deleted the date-type branch February 8, 2019 16:53

chriddyp mentioned this pull request Feb 8, 2019

data types architecture (sponsored: due feb 1) #166

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Datetime data type #364

Datetime data type #364

alexcjohnson commented Feb 7, 2019

alexcjohnson commented Feb 7, 2019

alexcjohnson Feb 7, 2019

Marc-Andre-Rivet Feb 7, 2019 •

edited

Loading

alexcjohnson Feb 7, 2019

Marc-Andre-Rivet Feb 7, 2019 •

edited

Loading

Marc-Andre-Rivet Feb 7, 2019

alexcjohnson Feb 7, 2019

Marc-Andre-Rivet Feb 7, 2019

alexcjohnson Feb 7, 2019

Marc-Andre-Rivet Feb 7, 2019

alexcjohnson Feb 8, 2019

Marc-Andre-Rivet Feb 7, 2019

alexcjohnson Feb 7, 2019

Marc-Andre-Rivet Feb 7, 2019

alexcjohnson Feb 8, 2019

Marc-Andre-Rivet commented Feb 7, 2019 •

edited

Loading

Marc-Andre-Rivet Feb 8, 2019 •

edited

Loading

alexcjohnson Feb 8, 2019

Marc-Andre-Rivet Feb 8, 2019 •

edited

Loading

Marc-Andre-Rivet Feb 8, 2019

alexcjohnson Feb 8, 2019

Marc-Andre-Rivet Feb 8, 2019

Marc-Andre-Rivet Feb 8, 2019

Marc-Andre-Rivet Feb 8, 2019

alexcjohnson Feb 8, 2019

Marc-Andre-Rivet Feb 8, 2019

Marc-Andre-Rivet left a comment

Datetime data type #364

Datetime data type #364

Conversation

alexcjohnson commented Feb 7, 2019

alexcjohnson commented Feb 7, 2019

Choose a reason for hiding this comment

Marc-Andre-Rivet Feb 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Marc-Andre-Rivet Feb 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Marc-Andre-Rivet commented Feb 7, 2019 • edited Loading

Marc-Andre-Rivet Feb 8, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Marc-Andre-Rivet Feb 8, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Marc-Andre-Rivet left a comment

Choose a reason for hiding this comment

Marc-Andre-Rivet Feb 7, 2019 •

edited

Loading

Marc-Andre-Rivet Feb 7, 2019 •

edited

Loading

Marc-Andre-Rivet commented Feb 7, 2019 •

edited

Loading

Marc-Andre-Rivet Feb 8, 2019 •

edited

Loading

Marc-Andre-Rivet Feb 8, 2019 •

edited

Loading