CZML extractor: timezone issues (clock.Interval and currentTime not tz-aware)

[kerel-fs]

🐞 Problem

The ISO8601 timestamps in the clock.interval and currentTime fields by the czml extractor are missing the 'Z' character indicating UTC. As a result, when the users locale is UTC+1 the CZMLWidget will start one hour too early (so that the satellites will only be visible after one hour passed in the simulation.

Example notebook

🖥 Envorionment

• pip freeze
  • poliastro==0.13.post0.dev0

💡 Possible solutions

• option A: Extend czml3.types.TimeInterval to have also a _value like IntervalValue has. Then use czml3.types.TimeInterval instead of czml3.types.IntervalValue in extract_czml.py#L174-L181:

  pckt = Preamble(
      name="document_packet",
      clock=IntervalValue(
          start=self.start_epoch.value,
          end=self.end_epoch.value,
          value=Clock(currentTime=self.start_epoch.value, multiplier=60),
      ),
  )

• option B: Copy datetime formatting code from czml3.types.TimeInterval into the relevant section in extract_czml.py

A similar issue was fixed in #783.

[astrojuanlu]

The problem seems to be that we are prematurely converting the dates to strings:

>>> import datetime as dt
>>> from astropy.time import Time
>>> start = dt.datetime.now(dt.timezone.utc)
>>> start_utc = Time(start, scale="utc"); start_utc
<Time object: scale='utc' format='datetime' value=2019-12-17 09:58:21.246200>
>>> Time(start_utc, format="isot").value
'2019-12-17T09:58:21.246'

poliastro/src/poliastro/czml/extract_czml.py

Lines 80 to 81 in 1ddf2d5

	self.start_epoch = Time(start_epoch, format="isot")
	self.end_epoch = Time(end_epoch, format="isot")

Which is probably not necessary, because TimeInterval has some extra logic to properly render itself using several datetime-like objects:

>>> start
datetime.datetime(2019, 12, 17, 9, 58, 21, 246200, tzinfo=datetime.timezone.utc)
>>> end
datetime.datetime(2019, 12, 17, 10, 58, 21, 246200, tzinfo=datetime.timezone.utc)
>>> Preamble(name="test", clock=IntervalValue(start=start, end=end, value=Clock(currentTime=start, multiplier=60)))
{
    "id": "document",
    "version": "1.0",
    "name": "test",
    "clock": {
        "interval": "2019-12-17T09:58:21Z/2019-12-17T10:58:21Z",
        "currentTime": "2019-12-17T09:58:21Z",
        "multiplier": 60,
        "range": "LOOP_STOP",
        "step": "SYSTEM_CLOCK_MULTIPLIER"
    }
}
>>> Preamble(name="test", clock=IntervalValue(start=Time(start, scale="utc"), end=Time(end, scale="utc"), value=Clock(currentTime=start, multiplier=60)))
{
    "id": "document",
    "version": "1.0",
    "name": "test",
    "clock": {
        "interval": "2019-12-17T09:58:21Z/2019-12-17T10:58:21Z",
        "currentTime": "2019-12-17T09:58:21Z",
        "multiplier": 60,
        "range": "LOOP_STOP",
        "step": "SYSTEM_CLOCK_MULTIPLIER"
    }
}

(unlike Clock, by the way:

>>> Clock(currentTime=Time(start, scale="utc"), multiplier=60)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/juanlu/.pyenv/versions/poliastro37_4/lib/python3.7/site-packages/czml3/base.py", line 31, in __repr__
    return self.dumps(indent=4)
  File "/home/juanlu/.pyenv/versions/poliastro37_4/lib/python3.7/site-packages/czml3/base.py", line 38, in dumps
    return json.dumps(self, *args, **kwargs)
  File "/home/juanlu/.pyenv/versions/3.7.4/lib/python3.7/json/__init__.py", line 238, in dumps
    **kw).encode(obj)
  File "/home/juanlu/.pyenv/versions/3.7.4/lib/python3.7/json/encoder.py", line 201, in encode
    chunks = list(chunks)
  File "/home/juanlu/.pyenv/versions/3.7.4/lib/python3.7/json/encoder.py", line 439, in _iterencode
    yield from _iterencode(o, _current_indent_level)
  File "/home/juanlu/.pyenv/versions/3.7.4/lib/python3.7/json/encoder.py", line 431, in _iterencode
    yield from _iterencode_dict(o, _current_indent_level)
  File "/home/juanlu/.pyenv/versions/3.7.4/lib/python3.7/json/encoder.py", line 405, in _iterencode_dict
    yield from chunks
  File "/home/juanlu/.pyenv/versions/3.7.4/lib/python3.7/json/encoder.py", line 438, in _iterencode
    o = _default(o)
  File "/home/juanlu/.pyenv/versions/poliastro37_4/lib/python3.7/site-packages/czml3/base.py", line 25, in default
    return super().default(o)
  File "/home/juanlu/.pyenv/versions/3.7.4/lib/python3.7/json/encoder.py", line 179, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type Time is not JSON serializable

)

Therefore, I suggest that we remove format="isot" or that we create our own "isotz" format subclassing this:

https://github.com/astropy/astropy/blob/efa27f063a0f33e7bd5be00cc434eec9ad45e8ea/astropy/time/formats.py#L1190-L1213

[sundeshgupta]

Can I take up this issue?

[astrojuanlu]

Hi @sundeshgupta, sure all yours!

[sundeshgupta]

Hi @Juanlu001 . Since I am a beginner here, I need some help solving this issue.
What I infer from your previous comment is that we should not convert dates to strings. Also, IntervalValue can accept start time and end time of type Time, and Clock currentTime parameter does not accept Time class object but can accept objects of datetime class. So, what I think could be simpler solution is something like this:

>>>Preamble(name = "test", clock = IntervalValue(start = start_epoch, end = end_epoch, value = Clock(currentTime = start_epoch.datetime, multiplier = 60)))
{
    "id": "document",
    "version": "1.0",
    "name": "test",
    "clock": {
        "interval": "2013-03-18T12:00:00Z/2013-03-18T23:59:35Z",
        "currentTime": "2013-03-18T06:30:00Z",
        "multiplier": 60,
        "range": "LOOP_STOP",
        "step": "SYSTEM_CLOCK_MULTIPLIER"
    }
}
>>> start_epoch
<Time object: scale='utc' format='iso' value=2013-03-18 12:00:00.000>

Any comments on this?

[sundeshgupta]

In case you missed my comment, could you please take a moment to look at my previous comment.

[astrojuanlu]

Hi @sundeshgupta, sorry for the delay!

If I understood correctly, you propose replacing this:

poliastro/src/poliastro/czml/extract_czml.py

Line 179 in 1ddf2d5

value=Clock(currentTime=self.start_epoch.value, multiplier=60),

with value=Clock(currentTime=self.start_epoch.datetime, multiplier=60), is that correct?

I think it would work as a workaround for the current limitations of the Clock and fix the issue, although we might later decide to fix czml3 upstream.

Do you want to open a pull request with this fix, including a unit test for it?

[sundeshgupta]

with value=Clock(currentTime=self.start_epoch.datetime, multiplier=60), is that correct?

Yes, this is what I propose.

Do you want to open a pull request with this fix, including a unit test for it?

Sure. I will open the pull request resolving this issue.

[astrojuanlu]

Fixed! If anybody wants to send a pull request to czml3 so that Clock supports Time objects in the same way TimeInterval does, go ahead!