-
Notifications
You must be signed in to change notification settings - Fork 507
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
Match WHO Dashboard - Add one day to stats #1921
base: master
Are you sure you want to change the base?
Conversation
Is this a timezone issue, a server issue, or a parsing issue? |
@matthewblain - timestamps are for midnight, so it's ambiguous which day it applies to. It looks as though the WHO dashboard uses the date of when the data was reported to WHO - this is different to the date on which the case or death occurred. Adding one day is consistent with this, so the data that appears suddenly gets the date on which it was reported. |
@@ -75,7 +75,13 @@ class _RecentNumbersBarGraphState extends State<RecentNumbersBarGraph> { | |||
final date = DateTime.fromMillisecondsSinceEpoch( | |||
widget.timeseries[index].epochMsec.toInt()); | |||
// Abbr to fit on single line: e.g. "Oct 18, 2020" | |||
final formattedDate = DateFormat.yMMMd().format(date); | |||
final formattedDate = DateFormat.yMMMd().format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extract this to a shared method to avoid duplicated code.
This should also be heavily commented to explain why it exists.
@matthewblain - you could make a case for fixing this on the server.... but that would break the data consistency between the ArcGIS output and the rest API. |
I guess I think it should just be defined what it is. It's a time_t variant--are they all actual times, or midnight in some timezone, I looked at a sample of data from last month and they all appear to be midnight UTC. DateTime.fromMillisecondsSinceEpoch will generate an item in the local timezone, though there is an option to have be UTC. Then formatdate does... something with it. What is the date on the WHO dashboard generated from? Does it take the UTC midnight time and generate a string date from that, then display that everywhere in the world? |
As a concrete example, for Nigeria, on the WHO dashboard web site, it shows 1145 new cases for Dec 18. This matches with data for timestamp 1608249600000. Which is Dec 18, 0:00 Zulu. Which also happens to be 2020-12-17T16:00:00 in Pacific time. Has this bug been verified with the device timezone set on both sides of UTC? |
This item has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
I just tested, and this is not the correct solution. It is a timezone issue. |
@matthewblain, do you think setting the timezone to UTC will work? |
Yes, it's quite possible using the isUtc argument on the constructor will do the trick:
) |
@matthewblain - I tip my hat to you. That would make more sense. Certainly, it shouldn't change depending on the local device time or time zone. |
I would've preferred that the server API used dates instead of timestamps to avoid any ambiguity.... but that's not important for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@theswerd - please look in to @matthewblain's comment about time zone. That is a plausible explanation to me. I don't know what time zone the data is in but I'll ask WHO.
@@ -75,7 +76,7 @@ class _RecentNumbersBarGraphState extends State<RecentNumbersBarGraph> { | |||
final date = DateTime.fromMillisecondsSinceEpoch( | |||
widget.timeseries[index].epochMsec.toInt()); | |||
// Abbr to fit on single line: e.g. "Oct 18, 2020" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move comment to the extension as the comment it common to both usages and not specific to line 78
@@ -0,0 +1,13 @@ | |||
import 'package:intl/intl.dart'; | |||
|
|||
extension WHOFormat on DateTime { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you wanted to write an extension ;-) I would've included the DateTime.fromMillisecondsSinceEpoch
step also as that's common to both usages.... and it's the msSinceEpoch that has midnight ambiguity. By the time it's converted to a date, it should be correct, so doing +1 in the format step doesn't fit so well to me.
I would've just extracted a single method and not bothered with a new file. We can move it to a separate file if something else uses it. I think though that this formatting may be specific to the WHO dashboard.... no idea if it extends to the
String whoDashboardDateFormat(int epochMS) {
var date = DateTime.fromMillisecondsSinceEpoch(epochMS);
// Timestamps are for midnight so the day it applies to is ambiguous.
// Add 1 day to match the WHO Dashboard: http://covid19.who.int
date.add(Duration(days: 1))
// Abbr to fit on single line: e.g. "Oct 18, 2020"
return DateFormat.yMMMd().format(date);
}
This item has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This item has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Closes #1917
Screenshots
How did you test the change?
curl
to a dev App Engine serverChecklist: