Bugfixes for ITRF transformation code #10
Conversation
| """ | ||
|
|
||
| def sinceEpoch(date): | ||
| # returns seconds since epoch | ||
| return time.mktime(date.timetuple()) | ||
| return calendar.timegm(date.timetuple()) |
There was a problem hiding this comment.
This was a subtle bug (thanks stackoverflow!). time.mktime assumes the local timezone, not UTC. Since all of our timestamps are UTC, calendar.timegm should be used instead.
Prior to this cahnge, a the UTC timestamp "1993-07-02 12:00:00" was being converted into decimal year 1993.4998858447489 instead of 1993.5.
You can actually replicate the correct behavior without changing the code by setting the TZ envvar to UTC:
$ python -c "from iceflow.itrf.converter import _datetime_to_decimal_year; import pandas as pd; print(_datetime_to_decimal_year(pd.to_datetime('1993-07-02 12:00:00')))"
1993.4998858447489
$ TZ=UTC python -c "from iceflow.itrf.converter import _datetime_to_decimal_year; import pandas as pd; print(_datetime_to_decimal_year(pd.to_datetime('1993-07-02 12:00:00')))"
1993.5
|
|
||
| # Before and after values were extracted from the synthetic dataset in the | ||
| # ITRF corrections notebook put together by Kevin. See: | ||
| # https://github.com/nsidc/NSIDC-Data-Tutorials/blob/a35203f18841456d258fac3a11dc04f44f839d9d/notebooks/iceflow/corrections.ipynb |
There was a problem hiding this comment.
I found the datetime conversion bug because the notebook hard-codes the decimal year as 1993.5 - it doesn't calculate it from a datetime like we normally do for real data.
| @@ -82,7 +84,7 @@ def transform_itrf( | |||
| # TODO: Should we create a new decimalyears when doing an epoch | |||
| # propagation since PROJ doesn't do this? | |||
|
|
|||
| lats, lons, elevs, _ = transformer.transform( | |||
| lons, lats, elevs, _ = transformer.transform( | |||
There was a problem hiding this comment.
This was my mistake when re-factoring the code I borrowed from valkyrie.
The code as currently written should be invariant with respect to the set timezone. Prior to the bugfix that led to this test, the result would be wrong when the local TZ was "America/Denver".
ensures that the env is cleaned up after testrun
|
|
||
| @pytest.mark.parametrize("timezone", ["America/Denver", "UTC"]) | ||
| def test__datetime_to_decimal_year(timezone, monkeypatch): | ||
| monkeypatch.setenv("TZ", timezone) |
No description provided.