Repository: planetarypy/pvl

Issue:
Returned Date/Time objects must be "aware"
The current (1.0.0-alpha2) implementation can return datetime objects that are either "naive" or "aware" (to use the terminology from the Python datetime library).

However, the PVL Specification clearly indicates in section 2.3.2.1.3 (Date/Time Value) that: 

> The Date/Time Value is a strict subset of the CCSDS ASCII Time Code recommendation (Reference [3]), in which all time is represented in Universal Coordinated Time, (i.e. Greenwich Mean Time). 

This means that any time the `pvl` library returns a datetime object, it should be an "aware" datetime object.

Discussion:

Patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index d153e89..6a3f189 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -3,6 +3,13 @@
 History
 -------
 
+1.0.0-alpha.6 (2020-07-27)
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+* Enforced that all datetime.time and datetime.datetime objects
+  returned should be timezone "aware."  This breaks 0.x functionality
+  where some were and some weren't.  Addresses #57.
+
+
 1.0.0-alpha.5 (2020-05-30)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
 * ISIS creates PVL text with unquoted plus signs ("+"), needed to adjust
diff --git a/pvl/__init__.py b/pvl/__init__.py
index c5662af..f1c6184 100755
--- a/pvl/__init__.py
+++ b/pvl/__init__.py
@@ -23,7 +23,7 @@ from ._collections import (
 
 __author__ = 'The pvl Developers'
 __email__ = 'trevor@heytrevor.com'
-__version__ = '1.0.0-alpha.5'
+__version__ = '1.0.0-alpha.6'
 __all__ = [
     'load',
     'loads',
diff --git a/pvl/decoder.py b/pvl/decoder.py
index 80a3610..d172312 100644
--- a/pvl/decoder.py
+++ b/pvl/decoder.py
@@ -193,8 +193,16 @@ class PVLDecoder(object):
         seconds.  However, the Python ``datetime`` classes don't
         support second values for more than 59 seconds.
 
+        Since the PVL Blue Book says that all PVl Date/Time Values
+        are represented in Universal Coordinated Time, then all
+        datetime objects that are returned datetime Python objects
+        should be timezone "aware."  A datetime.date object is always
+        "naive" but any datetime.time or datetime.datetime objects
+        returned from this function will be "aware."
+
         If a time with 60 seconds is encountered, it will not be
-        returned as a datetime object, but simply as a string.
+        returned as a datetime object (since that is not representable
+        via Python datetime objects), but simply as a string.
 
         The user can then then try and use the ``time`` module
         to parse this string into a ``time.struct_time``.  We
@@ -212,29 +220,46 @@ class PVLDecoder(object):
         numerical types, and do something useful with them.
         """
         try:
+            # datetime.date objects will always be naive, so just return:
             return for_try_except(ValueError, datetime.strptime,
                                   repeat(value),
                                   self.grammar.date_formats).date()
         except ValueError:
+            # datetime.time and datetime.datetime might be either:
+            d = None
             try:
-                return for_try_except(ValueError, datetime.strptime,
-                                      repeat(value),
-                                      self.grammar.time_formats).time()
+                d = for_try_except(ValueError, datetime.strptime,
+                                   repeat(value),
+                                   self.grammar.time_formats).time()
             except ValueError:
                 try:
-                    return for_try_except(ValueError, datetime.strptime,
-                                          repeat(value),
-                                          self.grammar.datetime_formats)
+                    d = for_try_except(ValueError, datetime.strptime,
+                                       repeat(value),
+                                       self.grammar.datetime_formats)
                 except ValueError:
                     pass
+            if d is not None:
+                if d.utcoffset() is None:
+                    return d.replace(tzinfo=timezone.utc)
+                else:
+                    return d
 
         # if we can regex a 60-second time, return str
+        if self.is_leap_seconds(value):
+            return str(value)
+        else:
+            raise ValueError
+
+    def is_leap_seconds(self, value: str) -> bool:
+        """Returns True if *value* is a time that matches the
+        grammar's definition of a leap seconds time (a time string with
+        a value of 60 for the seconds value).  False otherwise."""
         for r in (self.grammar.leap_second_Ymd_re,
                   self.grammar.leap_second_Yj_re):
             if r is not None and r.fullmatch(value) is not None:
-                return str(value)
-
-        raise ValueError
+                return True
+        else:
+            return False
 
     def decode_quantity(self, value, unit):
         """Returns a Python object that represents a value with
@@ -280,7 +305,7 @@ class ODLDecoder(PVLDecoder):
             # Otherwise ...
             match = re.fullmatch(r'(?P<dt>.+?)'  # the part before the sign
                                  r'(?P<sign>[+-])'  # required sign
-                                 r'(?P<hour>0?[1-9]|1[0-2])'  # 1 to 12
+                                 r'(?P<hour>0?[0-9]|1[0-2])'  # 0 to 12
                                  fr'(?:{self.grammar._M_frag})?',  # Minutes
                                  value)
             if match is not None:
diff --git a/pvl/encoder.py b/pvl/encoder.py
index bd5378a..f1ba1d4 100644
--- a/pvl/encoder.py
+++ b/pvl/encoder.py
@@ -669,7 +669,7 @@ class ODLEncoder(PVLEncoder):
 
         t = super().encode_time(value)
 
-        if value.tzinfo is None:
+        if value.tzinfo is None or value.tzinfo == 0:
             return t + 'Z'
         else:
             td_str = str(value.utcoffset())
diff --git a/setup.py b/setup.py
index f5fa90d..3a47230 100755
--- a/setup.py
+++ b/setup.py
@@ -12,7 +12,7 @@ history = open('HISTORY.rst').read().replace('.. :changelog:', '')
 
 setup(
     name='pvl',
-    version='1.0.0-alpha.5',
+    version='1.0.0-alpha.6',
     description='Python implementation of PVL (Parameter Value Language)',
     long_description=readme + '\n\n' + history,
     author='The PlanetaryPy Developers',
diff --git a/tox.ini b/tox.ini
index 70acb56..c8a1ce3 100644
--- a/tox.ini
+++ b/tox.ini
@@ -5,7 +5,7 @@ envlist = py36, py37, py38, flake8
 setenv =
     PYTHONPATH = {toxinidir}
 commands =
-    py.test tests
+    pytest
 deps =
     -r{toxinidir}/requirements.txt
 


Test patch:
diff --git a/tests/test_decoder.py b/tests/test_decoder.py
index c08da7f..c9ac534 100644
--- a/tests/test_decoder.py
+++ b/tests/test_decoder.py
@@ -90,15 +90,19 @@ class TestDecoder(unittest.TestCase):
                     self.assertEqual(p[1], self.d.decode_non_decimal(p[0]))
 
     def test_decode_datetime(self):
+        utc = datetime.timezone.utc
         for p in(('2001-01-01', datetime.date(2001, 1, 1)),
                  ('2001-027', datetime.date(2001, 1, 27)),
                  ('2001-027Z', datetime.date(2001, 1, 27)),
-                 ('23:45', datetime.time(23, 45)),
-                 ('01:42:57', datetime.time(1, 42, 57)),
-                 ('12:34:56.789', datetime.time(12, 34, 56, 789000)),
-                 ('2001-027T23:45', datetime.datetime(2001, 1, 27, 23, 45)),
-                 ('2001-01-01T01:34Z', datetime.datetime(2001, 1, 1, 1, 34)),
-                 ('01:42:57Z', datetime.time(1, 42, 57)),
+                 ('23:45', datetime.time(23, 45, tzinfo=utc)),
+                 ('01:42:57', datetime.time(1, 42, 57, tzinfo=utc)),
+                 ('12:34:56.789', datetime.time(12, 34, 56, 789000,
+                                                tzinfo=utc)),
+                 ('2001-027T23:45', datetime.datetime(2001, 1, 27, 23, 45,
+                                                      tzinfo=utc)),
+                 ('2001-01-01T01:34Z', datetime.datetime(2001, 1, 1, 1, 34,
+                                                         tzinfo=utc)),
+                 ('01:42:57Z', datetime.time(1, 42, 57, tzinfo=utc)),
                  ('2001-12-31T01:59:60.123Z', '2001-12-31T01:59:60.123Z'),
                  ('01:00:60', '01:00:60')):
             with self.subTest(pair=p):
@@ -149,6 +153,7 @@ class TestODLDecoder(unittest.TestCase):
 
     def test_decode_datetime(self):
         try:
+            utc = datetime.timezone.utc
             from dateutil import tz
             tz_plus_7 = tz.tzoffset('+7', datetime.timedelta(hours=7))
 
@@ -156,19 +161,21 @@ class TestODLDecoder(unittest.TestCase):
                       ('1990-158', datetime.date(1990, 6, 7)),
                       ('2001-001', datetime.date(2001, 1, 1)),
                       ('2001-01-01', datetime.date(2001, 1, 1)),
-                      ('12:00', datetime.time(12)),
-                      ('12:00:45', datetime.time(12, 0, 45)),
-                      ('12:00:45.4571', datetime.time(12, 0, 45, 457100)),
-                      ('15:24:12Z', datetime.time(15, 24, 12)),
+                      ('12:00', datetime.time(12, tzinfo=utc)),
+                      ('12:00:45', datetime.time(12, 0, 45, tzinfo=utc)),
+                      ('12:00:45.4571', datetime.time(12, 0, 45, 457100,
+                                                      tzinfo=utc)),
+                      ('15:24:12Z', datetime.time(15, 24, 12, tzinfo=utc)),
                       ('01:12:22+07',
                        datetime.time(1, 12, 22, tzinfo=tz_plus_7)),
                       ('01:12:22+7',
                        datetime.time(1, 12, 22, tzinfo=tz_plus_7)),
                       ('01:10:39.4575+07',
                        datetime.time(1, 10, 39, 457500, tzinfo=tz_plus_7)),
-                      ('1990-07-04T12:00', datetime.datetime(1990, 7, 4, 12)),
+                      ('1990-07-04T12:00', datetime.datetime(1990, 7, 4, 12,
+                                                             tzinfo=utc)),
                       ('1990-158T15:24:12Z',
-                       datetime.datetime(1990, 6, 7, 15, 24, 12)),
+                       datetime.datetime(1990, 6, 7, 15, 24, 12, tzinfo=utc)),
                       ('2001-001T01:10:39+7',
                        datetime.datetime(2001, 1, 1, 1, 10, 39,
                                          tzinfo=tz_plus_7)),
diff --git a/tests/test_pvl.py b/tests/test_pvl.py
index 33e2991..3b94af1 100755
--- a/tests/test_pvl.py
+++ b/tests/test_pvl.py
@@ -526,6 +526,7 @@ def test_dates():
     label = pvl.loads(some_pvl)
 
     tz_plus_7 = datetime.timezone(datetime.timedelta(hours=7))
+    utc = datetime.timezone.utc
 
     assert isinstance(label['date1'], datetime.date)
     assert label['date1'] == datetime.date(1990, 7, 4)
@@ -540,25 +541,26 @@ def test_dates():
     assert label['date4'] == datetime.date(2001, 1, 1)
 
     assert isinstance(label['time1'], datetime.time)
-    assert label['time1'] == datetime.time(12)
+    assert label['time1'] == datetime.time(12, tzinfo=utc)
 
     assert isinstance(label['time_s'], datetime.time)
-    assert label['time_s'] == datetime.time(12, 0, 45)
+    assert label['time_s'] == datetime.time(12, 0, 45, tzinfo=utc)
 
     assert isinstance(label['time_s_float'], datetime.time)
-    assert label['time_s_float'] == datetime.time(12, 0, 45, 457100)
+    assert label['time_s_float'] == datetime.time(12, 0, 45, 457100, tzinfo=utc)
 
     assert isinstance(label['time_tz1'], datetime.time)
-    assert label['time_tz1'] == datetime.time(15, 24, 12)
+    assert label['time_tz1'] == datetime.time(15, 24, 12, tzinfo=utc)
 
     assert isinstance(label['time_tz2'], datetime.time)
     assert label['time_tz2'] == datetime.time(1, 12, 22, tzinfo=tz_plus_7)
 
     assert isinstance(label['datetime1'], datetime.datetime)
-    assert label['datetime1'] == datetime.datetime(1990, 7, 4, 12)
+    assert label['datetime1'] == datetime.datetime(1990, 7, 4, 12, tzinfo=utc)
 
     assert isinstance(label['datetime2'], datetime.datetime)
-    assert label['datetime2'] == datetime.datetime(1990, 6, 7, 15, 24, 12)
+    assert label['datetime2'] == datetime.datetime(1990, 6, 7, 15, 24, 12,
+                                                   tzinfo=utc)
 
     assert isinstance(label['time_tz3'], datetime.time)
     assert label['time_tz3'] == datetime.time(1, 12, 22, tzinfo=tz_plus_7)