Skip to content
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

Timezones #6822

Closed
wants to merge 18 commits into from
Closed

Timezones #6822

wants to merge 18 commits into from

Conversation

sixlighthouses
Copy link
Contributor

@sixlighthouses sixlighthouses commented Jul 12, 2023

What this PR does

Fixes #6817

Add the ability to set a timeZone offset and a dateFormat

item example

    { 
      "id": "IUpoiafh",
      "name": "Landsat 5-9 Collection 2",
      "layers": "landsat5_c2l2_sr",
      "url": "https://ows.datacubechile.cl/wms",
      "type": "wms",
      "opacity": 1,
      "timeZone": "-3:00",
      "dateFormat": "isoDateTime"
    }

Created new trait class which extends ModelTraits
DateTimeTraits

has 2 traits

timeZone which takes an UTC offset with the format "+10:00"
dateFormat see useage at -- https://github.com/felixge/node-dateformat

Test me

http://ci.terria.io/timezones/#clean&https://gist.githubusercontent.com/sixlighthouses/a9d1e72fba0002caa068dd90418be5bc/raw/e7363a308f156e00f637b6bfa81ade10824b95ec/test.json

View the layers

  • Landsat 5-9 IsoDate with timezone -03:00
  • Landsat 5-9 IsoDateTime no timezone
  • Landsat 5-9 String format no timezone
  • Landsat 5-9 String format with timezone

The display of each of the layers datetime data in the workbench and timeline are defined by the following config

{ 
      "id": "IUpoiafh",
      "name": "Landsat 5-9 IsoDate with timezone -03:00",
      "layers": "landsat5_c2l2_sr",
      "url": "https://ows.datacubechile.cl/wms",
      "type": "wms",
      "opacity": 1,
      "dateFormat": "isoDate",
      "timeZone": "-03:00"
    },
    { 
      "id": "e43c635f-aa1e-4d16-9777-b8484ea10854",
      "name": "Landsat 5-9 IsoDateTime no timezone",
      "layers": "landsat5_c2l2_sr",
      "url": "https://ows.datacubechile.cl/wms",
      "type": "wms",
      "opacity": 1,
      "dateFormat": "isoDateTime"
    },
    { 
      "id": "5281d54a-5d05-4033-aa4a-63a0cbde402f",
      "name": "Landsat 5-9 String format no timezone",
      "layers": "landsat5_c2l2_sr",
      "url": "https://ows.datacubechile.cl/wms",
      "type": "wms",
      "opacity": 1,
      "dateFormat": "dddd, mmmm dS, yyyy, h:MM:ss TT"
    },
    { 
      "id": "a5649b8e-e114-44fd-bb84-a0fad1f63b25",
      "name": "Landsat 5-9 String format with timezone",
      "layers": "landsat5_c2l2_sr",
      "url": "https://ows.datacubechile.cl/wms",
      "type": "wms",
      "opacity": 1,
      "dateFormat": "dddd, mmmm dS, yyyy, h:MM:ss TT",
      "timeZone": "-03:00"
    },

Checklist

  • There are unit tests to verify my changes are correct or unit tests aren't applicable (if so, write quick reason why unit tests don't exist)
  • I've updated relevant documentation in doc/.
  • I've updated CHANGES.md with what I changed.
  • I've provided instructions in the PR description on how to test this PR.

Copy link
Member

@steve9164 steve9164 left a comment

Choose a reason for hiding this comment

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

This approach looks good for displaying time correctly based on dataset. I remember some conversation about timezones and DEA's imagery filtering - does this PR solve the problem, where the date is different due to timezones and so dates don't line up with those in data_available_for_dates from DEA and so the imagery fetched is wrong? Or is it not an issue, or should be dealt with separately?

It'd also be good to get some test cases, where timeZone is set differently.

I've left a few comments for minor improvements.

lib/ReactViews/BottomDock/Timeline/CesiumTimeline.jsx Outdated Show resolved Hide resolved
lib/ReactViews/BottomDock/Timeline/Timeline.jsx Outdated Show resolved Hide resolved
Comment on lines 1 to 6
export function getOffsetMinutes(timeZone: string): number {
const [hoursString, minutesString] = timeZone.split(":");
const hours = parseInt(hoursString);
const minutes = parseInt(minutesString);
return hours * 60 + minutes;
}
Copy link
Member

Choose a reason for hiding this comment

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

This fails when a timeZone of "+11" is used. I think it'd be good to support that form of timezone too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added regex check, so the function now checks for the timeZone to be in the format +/- HH:MM or +/-HH if not it will return 0 in which case no offset will be applied

@steve9164
Copy link
Member

steve9164 commented Aug 2, 2023

Is it also still possible for people from different time zones to have different dates appear (see comment on opendatacube)?

Fang: And we’d like users in US to be able to see the same time stamp as users in Africa. Eg we have a product generated for year 2020, with a time stamp of 2020-01-01. We’d like users everywhere to see it with time stamp 2020-01-01, not some users seeing 2019-12-31

Also, on a similar track, my earlier question below:

I remember some conversation about timezones and DEA's imagery filtering - does this PR solve the problem, where the date is different due to timezones and so dates don't line up with those in data_available_for_dates from DEA and so the imagery fetched is wrong? Or is it not an issue, or should be dealt with separately?

I think if you set a timezone of +3 on a dataset, the dataset says it has imagery for 2020-01-01 and you access it from the US (-7) you'll get the wrong date displaying - even after this PR. That's a difficult problem.

@sixlighthouses
Copy link
Contributor Author

you're correct Crispy, this PR does not address that issue. Because we set that as a Date object the browser will always interpret it to local time. If we wanted it to be displayed as it is delivered i.e not interpreted to local time, we could treat is a string in the display. Perhaps have a trait "staticTime" (names are hard) and if its true, display the string, not the date object.

@AnaBelgun AnaBelgun assigned sixlighthouses and unassigned steve9164 Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add timezone override trait
3 participants