Skip to content

use UTC milliseconds for internal representation of dates #1172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 64 commits into from

Conversation

alexcjohnson
Copy link
Collaborator

@etpinard this builds on the ISO-8601 PR #1158 so note the base branch but we can merge directly to master if #1158 goes in first.

Changes our internal date linearization to use UTC rather than local milliseconds. Every day on a Plotly graph will now be 24 hours long, with no daylight shifts.

Note that for backward compatibility I still have us treat millisecond values (in ranges etc) as they were calculated previously, in local time, and shift them to UTC. I do the same thing with js date objects, on the theory that users generally construct these by using new Date(y, m, d, H, M, S, ms) with the date/time components they expect to see on the final graph.

If and when we add support for explicit timezone settings, we can do smarter things for users who specify a timezone, but I think this should remain the default behavior.

Fixes #171 and #811

etpinard and others added 27 commits November 15, 2016 16:33
remove commented out code about 3d pie chart attributes
fix annotation category positioning
If we play with timestamps, we have to leave range bounds integers
- so that zoomed ranges are computed consistently
  for all axis types
- so that date axis ranges can be tested with some tolerance
* Fixed incorrect bar size in the hover labels of stacked bars.

Fixes #1157
- looks like the CI machine has less precision
  than my laptop
tasks: fixup main bundle URLs
We still interpret JS date objects according to the date and hour
they have in the local timezone, and for backward compatibility
we shift milliseconds (in ranges etc) by the local/UTC offset.
@@ -489,7 +489,7 @@ describe('Test shapes', function() {
title: 'linked to date and category axes',
xaxis: {
type: 'date',
range: ['2000-01-01', (new Date(2000, 1, 2)).getTime()]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this old format for date axis range was mucking up the test code but not affecting the code being tested (because that's all after supplyDefaults cleans these) so it was useless.

rreusser and others added 26 commits November 21, 2016 13:18
Add check for failed binding comparison
- allow , and spaces in middle of numeric strings
- and allow only ' ' in middle instead of all space characters
and test calc of uniform AND nonuniform bins
fix regression in lib/clean_number (alternate)
@alexcjohnson
Copy link
Collaborator Author

ugh, this didn't like separately merging master vs date-iso8601 - I'll just reopen this PR after we merge that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants