Repository: planetarypy/pvl

Issue:
ISIS PVL strings with + characters in string values are not supported
### OS
MAC OS 10.13

### Version
master @ be90a3e5425875828ca70e8fc66c67992559c64c

### Bug
ISIS allows for + characters in string values, even though the general spec. does not. Attempting to read in an ISIS PVL with a + causes an error.

### To reproduce
This is just a snippet of the full Cube label. I can provide that if needed too.
```
import pvl
pvl.loads("""
Object = NaifKeywords
  BODY_CODE                       = 499
  BODY499_RADII                   = (3396.19, 3396.19, 3376.2)
  BODY_FRAME_CODE                 = 10014
  INS-143400_SWAP_OBSERVER_TARGET = TRUE
  INS-143400_LIGHTTIME_CORRECTION = LT+S
  INS-143400_LT_SURFACE_CORRECT   = TRUE
  INS-143400_FOCAL_LENGTH         = 874.9
  INS-143400_PIXEL_PITCH          = 0.01
  INS-143400_TRANSX               = (0.0, 0.01, 0.0)
  INS-143400_TRANSY               = (0.0, 0.0, 0.01)
  INS-143400_ITRANSS              = (0.0, 100.0, 0.0)
  INS-143400_ITRANSL              = (0.0, 0.0, 100.0)
  INS-143400_BORESIGHT_SAMPLE     = 1024.5
  INS-143400_BORESIGHT_LINE       = 1024.5
  INS-143400_OD_A1_CORR           = (0.0037613053094827, -0.013415415606581,
                                     -1.86749521007237e-05, 1.0002135268184,
                                     -4.32362371703953e-04,
                                     -9.48065735350123e-04)
  INS-143400_OD_A2_CORR           = (9.9842559363676e-05, 0.0037354370795816,
                                     -0.013329991887393, -2.15311328389359e-04,
                                     0.99529601553729, -0.018354271771078)
  INS-143400_OD_A3_CORR           = (-3.13320167004204e-05,
                                     -7.35655125749807e-06,
                                     -1.57664245066771e-05, 0.0037354946543915,
                                     -0.014167194693093, 1.0)
  INS-143400_OD_A1_DIST           = (0.0021365879556062, -0.007117857650642,
                                     1.10355974742147e-05, 0.57360718262538,
                                     2.50884350194894e-04,
                                     5.50623913037132e-04)
  INS-143400_OD_A2_DIST           = (-5.69725741015406e-05,
                                     0.0021515590567915, -0.0071639299176719,
                                     1.24152787728634e-04, 0.57645954439243,
                                     0.010576940564854)
  INS-143400_OD_A3_DIST           = (1.78250771483506e-05,
                                     4.24592743471094e-06,
                                     9.51220699036653e-06, 0.0021515842542074,
                                     -0.0066835595774833, 0.57374154097161)
  INS-143400_FILTER_SAMPLES       = 2048.0
  INS-143400_FILTER_LINES         = 2048.0
End_Object
End
""")
```

Raises the following exception
```
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
~/miniconda3/envs/ale_pvl_test/lib/python3.7/site-packages/pvl-1.0.0a2-py3.7.egg/pvl/lexer.py in lexer(s, g, d)
    429                     yield None
--> 430                     t = yield(t)
    431                 lexeme = ''

ValueError: Expecting an Aggregation Block, an Assignment Statement, or an End Statement, but found "+" 

During handling of the above exception, another exception occurred:

LexerError                                Traceback (most recent call last)
<ipython-input-4-601e6f5d514f> in <module>
     43 End_Object
     44 End
---> 45 """)

~/miniconda3/envs/ale_pvl_test/lib/python3.7/site-packages/pvl-1.0.0a2-py3.7.egg/pvl/__init__.py in loads(s, parser, grammar, decoder, **kwargs)
    148         raise TypeError('The parser must be an instance of pvl.PVLParser.')
    149 
--> 150     return parser.parse(s)
    151 
    152 

~/miniconda3/envs/ale_pvl_test/lib/python3.7/site-packages/pvl-1.0.0a2-py3.7.egg/pvl/parser.py in parse(self, s)
    764         self.doc = nodash
    765 
--> 766         return super().parse(nodash)
    767 
    768     def parse_module_post_hook(self, module, tokens: abc.Generator):

~/miniconda3/envs/ale_pvl_test/lib/python3.7/site-packages/pvl-1.0.0a2-py3.7.egg/pvl/parser.py in parse(self, s)
    186         """Converts the string, *s* to a PVLModule."""
    187         tokens = self.lexer(s, g=self.grammar, d=self.decoder)
--> 188         module = self.parse_module(tokens)
    189         module.errors = sorted(self.errors)
    190         return module

~/miniconda3/envs/ale_pvl_test/lib/python3.7/site-packages/pvl-1.0.0a2-py3.7.egg/pvl/parser.py in parse_module(self, tokens)
    254         t = next(tokens)
    255         tokens.throw(ValueError,
--> 256                      'Expecting an Aggregation Block, an Assignment '
    257                      'Statement, or an End Statement, but found '
    258                      f'"{t}" ')

~/miniconda3/envs/ale_pvl_test/lib/python3.7/site-packages/pvl-1.0.0a2-py3.7.egg/pvl/lexer.py in lexer(s, g, d)
    434 
    435         except ValueError as err:
--> 436             raise LexerError(err, s, i, lexeme)

LexerError: (LexerError(...), 'Expecting an Aggregation Block, an Assignment Statement, or an End Statement, but found "+" : line 7 column 39 (char 247)')

```

Discussion:

Patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index 3db4c9a..d153e89 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -3,6 +3,18 @@
 History
 -------
 
+1.0.0-alpha.5 (2020-05-30)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+* ISIS creates PVL text with unquoted plus signs ("+"), needed to adjust
+  the ISISGrammar and OmniGrammar objects to parse this properly (#59).
+* In the process of doing so, realized that we have some classes that
+  optionally take a grammar and a decoder, and if they aren't given, to default.
+  However, a decoder *has* a grammar object, so if a grammar isn't provided, but
+  a decoder is, the grammar should be taken from the decoder, otherwise you
+  could get confusing behavior.
+* Updated pvl_validate to be explicit about these arguments.
+* Added a --version argument to both pvl_translate and pvl_validate.
+
 1.0.0.-alpha.4 (2020-05-29)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
 * Added the pvl.loadu() function as a convenience function to load PVL text from
diff --git a/pvl/__init__.py b/pvl/__init__.py
index 468d964..c5662af 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.4'
+__version__ = '1.0.0-alpha.5'
 __all__ = [
     'load',
     'loads',
diff --git a/pvl/encoder.py b/pvl/encoder.py
index 4d24067..bd5378a 100644
--- a/pvl/encoder.py
+++ b/pvl/encoder.py
@@ -28,6 +28,9 @@ class PVLEncoder(object):
     """An encoder based on the rules in the CCSDS-641.0-B-2 'Blue Book'
     which defines the PVL language.
 
+    :param grammar: A pvl.grammar object, if None or not specified, it will
+                    be set to the grammar parameter of *decoder* (if
+                    *decoder* is not None) or will default to PVLGrammar().
     :param grammar: defaults to pvl.grammar.PVLGrammar().
     :param decoder: defaults to pvl.decoder.PVLDecoder().
     :param indent: specifies the number of spaces that will be used to
@@ -53,7 +56,10 @@ class PVLEncoder(object):
                  newline: str = '\n'):
 
         if grammar is None:
-            self.grammar = PVLGrammar()
+            if decoder is not None:
+                self.grammar = decoder.grammar
+            else:
+                self.grammar = PVLGrammar()
         elif isinstance(grammar, PVLGrammar):
             self.grammar = grammar
         else:
diff --git a/pvl/grammar.py b/pvl/grammar.py
index de4b975..52dbcfc 100644
--- a/pvl/grammar.py
+++ b/pvl/grammar.py
@@ -8,6 +8,7 @@
 # top level of this library.
 
 import re
+from collections import abc
 
 
 class PVLGrammar():
@@ -217,6 +218,21 @@ class ISISGrammar(PVLGrammar):
     object_pref_keywords = ('Object', 'End_Object')
     object_keywords = {'OBJECT': 'END_OBJECT'}
 
+    def __init__(self):
+        # ISIS allows for + characters in Unquoted String values.
+        self.reserved_characters = tuple(
+            self.adjust_reserved_characters(self.reserved_characters)
+        )
+
+    @staticmethod
+    def adjust_reserved_characters(chars: abc.Iterable):
+        # ISIS allows for + characters in Unquoted String values.
+        # Removing the plus from the reserved characters allows for
+        # that, but might lead to other parsing errors, so be on the lookout.
+        rc = list(chars)
+        rc.remove("+")
+        return rc
+
 
 class OmniGrammar(PVLGrammar):
     """A broadly permissive grammar.
@@ -243,3 +259,10 @@ class OmniGrammar(PVLGrammar):
                                    fr'(?P<radix>[2-9]|1[0-6])#{_ss}')
     nondecimal_re = re.compile(nondecimal_pre_re.pattern +
                                r'(?P<non_decimal>[0-9|A-F|a-f]+)#')
+
+    def __init__(self):
+        # Handle the fact that ISIS writes out unquoted plus signs.
+        # See ISISGrammar for details.
+        self.reserved_characters = tuple(
+            ISISGrammar.adjust_reserved_characters(self.reserved_characters)
+        )
diff --git a/pvl/parser.py b/pvl/parser.py
index 2066341..6006c58 100644
--- a/pvl/parser.py
+++ b/pvl/parser.py
@@ -124,7 +124,10 @@ class PVLParser(object):
     """A parser based on the rules in the CCSDS-641.0-B-2 'Blue Book'
     which defines the PVL language.
 
-    :param grammar: defaults to :class:`pvl.grammar.OmniGrammar()`.
+    :param grammar: A pvl.grammar object, if None or not specified, it will
+                    be set to the grammar parameter of *decoder* (if
+                    *decoder* is not None) or will default to
+                    :class:`pvl.grammar.OmniGrammar()`.
     :param decoder: defaults to :class:`pvl.decoder.OmniDecoder()`.
     :param lexer_fn: must be a lexer function that takes a ``str``,
         a grammar, and a decoder, as :func:`pvl.lexer.lexer()` does,
@@ -154,7 +157,10 @@ class PVLParser(object):
             self.lexer = lexer_fn
 
         if grammar is None:
-            self.grammar = OmniGrammar()
+            if decoder is not None:
+                self.grammar = decoder.grammar
+            else:
+                self.grammar = OmniGrammar()
         elif isinstance(grammar, PVLGrammar):
             self.grammar = grammar
         else:
diff --git a/pvl/pvl_translate.py b/pvl/pvl_translate.py
index e300fbd..9db2c01 100644
--- a/pvl/pvl_translate.py
+++ b/pvl/pvl_translate.py
@@ -66,6 +66,7 @@ def arg_parser(formats):
                         default=sys.stdout,
                         help='file to write translated PVL to, defaults '
                         'to STDOUT.')
+    parser.add_argument("--version", action="version", version=pvl.__version__)
     return parser
 
 
diff --git a/pvl/pvl_validate.py b/pvl/pvl_validate.py
index 6110565..224705d 100644
--- a/pvl/pvl_validate.py
+++ b/pvl/pvl_validate.py
@@ -30,32 +30,57 @@ from .parser import ParseError, PVLParser, ODLParser, OmniParser
 from .decoder import PVLDecoder, ODLDecoder, OmniDecoder
 from .encoder import PVLEncoder, ODLEncoder, ISISEncoder, PDSLabelEncoder
 
-dialects = OrderedDict(PDS3=dict(parser=ODLParser(),
-                                 grammar=ODLGrammar(),
-                                 decoder=ODLDecoder(),
-                                 encoder=PDSLabelEncoder()),
-                       ODL=dict(parser=ODLParser(),
-                                grammar=ODLGrammar(),
-                                decoder=ODLDecoder(),
-                                encoder=ODLEncoder()),
-                       PVL=dict(parser=PVLParser(),
-                                grammar=PVLGrammar(),
-                                decoder=PVLDecoder(),
-                                encoder=PVLEncoder()),
-                       ISIS=dict(parser=OmniParser(),
-                                 grammar=ISISGrammar(),
-                                 decoder=OmniDecoder(),
-                                 encoder=ISISEncoder()),
-                       Omni=dict(parser=OmniParser(),
-                                 grammar=OmniGrammar(),
-                                 decoder=OmniDecoder(),
-                                 encoder=PVLEncoder()))
+# Some assembly required for the dialects.
+# We are going to be explicit here, because these arguments are
+# are different than the defaults for these classes, especially for the
+# parsers and decoders, as we want to be strict and not permissive here.
+_pvl_g = PVLGrammar()
+_pvl_d = PVLDecoder(grammar=_pvl_g)
+_odl_g = ODLGrammar()
+_odl_d = ODLDecoder(grammar=_odl_g)
+_odl_p = ODLParser(grammar=_odl_g, decoder=_odl_d)
+_isis_g = ISISGrammar()
+_isis_d = OmniDecoder(grammar=_isis_g)
+_omni_g = OmniGrammar()
+_omni_d = OmniDecoder(grammar=_omni_g)
+
+dialects = OrderedDict(
+    PDS3=dict(
+        parser=_odl_p,
+        grammar=_odl_g,
+        decoder=_odl_d,
+        encoder=PDSLabelEncoder(grammar=_odl_g, decoder=_odl_d)
+    ),
+    ODL=dict(
+        parser=_odl_p,
+        grammar=_odl_g,
+        decoder=_odl_d,
+        encoder=ODLEncoder(grammar=_odl_g, decoder=_odl_d)
+    ),
+    PVL=dict(
+        parser=PVLParser(grammar=_pvl_g, decoder=_pvl_d),
+        grammar=_pvl_g,
+        decoder=_pvl_d,
+        encoder=PVLEncoder(grammar=_pvl_g, decoder=_pvl_d)
+    ),
+    ISIS=dict(
+        parser=OmniParser(grammar=_isis_g, decoder=_isis_d),
+        grammar=_isis_g,
+        decoder=_isis_d,
+        encoder=ISISEncoder(grammar=_isis_g, decoder=_isis_d)
+    ),
+    Omni=dict(
+        parser=OmniParser(grammar=_omni_g, decoder=_omni_d),
+        grammar=_omni_g,
+        decoder=_omni_d,
+        encoder=PVLEncoder(grammar=_omni_g, decoder=_omni_d)))
 
 
 def arg_parser():
     p = argparse.ArgumentParser(description=__doc__)
     p.add_argument('-v', '--verbose', action='count', default=0,
                    help='Will report the errors that are encountered.')
+    p.add_argument("--version", action="version", version=pvl.__version__)
     p.add_argument('file', nargs='+',
                    help='file containing PVL text to validate.')
     return p
@@ -79,6 +104,8 @@ def main():
         results_list.append((f, results))
 
     # Writing the flavors out again to preserve order.
+    if args.verbose > 0:
+        print(f"pvl library version: {pvl.__version__}")
     print(report(results_list, list(dialects.keys())))
     return
 
diff --git a/pvl/token.py b/pvl/token.py
index 42fa002..8554198 100644
--- a/pvl/token.py
+++ b/pvl/token.py
@@ -16,9 +16,12 @@ class Token(str):
 
     :var content: A string that is the Token text.
 
-    :var grammar: A pvl.grammar object, defaults to PVLGrammar().
+    :var grammar: A pvl.grammar object, if None or not specified, it will
+                  be set to the grammar parameter of *decoder* (if
+                  *decoder* is not None) or will default to PVLGrammar().
 
-    :var decoder: A pvl.decoder object, defaults to PVLDecoder().
+    :var decoder: A pvl.decoder object, defaults to
+                  PVLDecoder(grammar=*grammar*).
 
     :var pos: Integer that describes the starting position of this
               Token in the source string, defaults to zero.
@@ -29,18 +32,21 @@ class Token(str):
 
     def __init__(self, content, grammar=None, decoder=None, pos=0):
         if grammar is None:
-            self.grammar = PVLGrammar()
+            if decoder is not None:
+                self.grammar = decoder.grammar
+            else:
+                self.grammar = PVLGrammar()
         elif isinstance(grammar, PVLGrammar):
             self.grammar = grammar
         else:
-            raise Exception
+            raise TypeError("The grammar object is not of type PVLGrammar.")
 
         if decoder is None:
-            self.decoder = PVLDecoder(self.grammar)
+            self.decoder = PVLDecoder(grammar=self.grammar)
         elif isinstance(decoder, PVLDecoder):
             self.decoder = decoder
         else:
-            raise Exception
+            raise TypeError("The decoder object is not of type PVLDecoder.")
 
         self.pos = pos
 
@@ -128,9 +134,9 @@ class Token(str):
             return True
 
         for ws in reversed(self.grammar.whitespace):
-            self = self.replace(ws, ' ')
+            temp = self.replace(ws, ' ')
 
-        return all(t.is_comment() for t in self.split())
+        return all(t.is_comment() for t in temp.split())
 
     def is_comment(self) -> bool:
         """Return true if the Token is a comment according to the
diff --git a/setup.py b/setup.py
index dab918c..f5fa90d 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.4',
+    version='1.0.0-alpha.5',
     description='Python implementation of PVL (Parameter Value Language)',
     long_description=readme + '\n\n' + history,
     author='The PlanetaryPy Developers',


Test patch:
diff --git a/tests/data/isis_naif.txt b/tests/data/isis_naif.txt
new file mode 100644
index 0000000..5a8c7a1
--- /dev/null
+++ b/tests/data/isis_naif.txt
@@ -0,0 +1,43 @@
+Object = NaifKeywords
+  BODY_CODE                       = 499
+  BODY499_RADII                   = (3396.19, 3396.19, 3376.2)
+  BODY_FRAME_CODE                 = 10014
+  INS-143400_SWAP_OBSERVER_TARGET = TRUE
+  INS-143400_LIGHTTIME_CORRECTION = LT+S
+  INS-143400_LT_SURFACE_CORRECT   = TRUE
+  INS-143400_FOCAL_LENGTH         = 874.9
+  INS-143400_PIXEL_PITCH          = 0.01
+  INS-143400_TRANSX               = (0.0, 0.01, 0.0)
+  INS-143400_TRANSY               = (0.0, 0.0, 0.01)
+  INS-143400_ITRANSS              = (0.0, 100.0, 0.0)
+  INS-143400_ITRANSL              = (0.0, 0.0, 100.0)
+  INS-143400_BORESIGHT_SAMPLE     = 1024.5
+  INS-143400_BORESIGHT_LINE       = 1024.5
+  INS-143400_OD_A1_CORR           = (0.0037613053094827, -0.013415415606581,
+                                     -1.86749521007237e-05, 1.0002135268184,
+                                     -4.32362371703953e-04,
+                                     -9.48065735350123e-04)
+  INS-143400_OD_A2_CORR           = (9.9842559363676e-05, 0.0037354370795816,
+                                     -0.013329991887393, -2.15311328389359e-04,
+                                     0.99529601553729, -0.018354271771078)
+  INS-143400_OD_A3_CORR           = (-3.13320167004204e-05,
+                                     -7.35655125749807e-06,
+                                     -1.57664245066771e-05, 0.0037354946543915,
+                                     -0.014167194693093, 1.0)
+  INS-143400_OD_A1_DIST           = (0.0021365879556062, -0.007117857650642,
+                                     1.10355974742147e-05, 0.57360718262538,
+                                     2.50884350194894e-04,
+                                     5.50623913037132e-04)
+  INS-143400_OD_A2_DIST           = (-5.69725741015406e-05,
+                                     0.0021515590567915, -0.0071639299176719,
+                                     1.24152787728634e-04, 0.57645954439243,
+                                     0.010576940564854)
+  INS-143400_OD_A3_DIST           = (1.78250771483506e-05,
+                                     4.24592743471094e-06,
+                                     9.51220699036653e-06, 0.0021515842542074,
+                                     -0.0066835595774833, 0.57374154097161)
+  INS-143400_FILTER_SAMPLES       = 2048.0
+  INS-143400_FILTER_LINES         = 2048.0
+End_Object
+End
+
diff --git a/tests/test_pvl.py b/tests/test_pvl.py
index 1f4048e..33e2991 100755
--- a/tests/test_pvl.py
+++ b/tests/test_pvl.py
@@ -764,6 +764,9 @@ def test_isis_output():
     label = pvl.load(os.path.join(DATA_DIR, 'isis_output.txt'))
     assert label['Results']['TotalPixels'] == 2048000
 
+    naif = pvl.load(os.path.join(DATA_DIR, 'isis_naif.txt'))
+    assert naif["NaifKeywords"]['INS-143400_LIGHTTIME_CORRECTION'] == "LT+S"
+
 
 def test_cube_label():
     with open(os.path.join(DATA_DIR, 'pattern.cub'), 'rb') as fp: