-
Notifications
You must be signed in to change notification settings - Fork 21
Description
Description
A hashmark-style/single-line comment that ends with a dash (-) character causes the Omni reader to behave unexpectedly.
-
If the comment precedes the start of an aggregate parameter, the reader seems unable to find the closing delimiter. The exception message clearly indicates it's there, however, but indicates the parser was expecting an assignment on that particular line (the line that closes the aggregate).
- (UPDATE: I have realized this is actually the same problem as the one below: the parameter following the comment seems not to have been parsed. The exception isn't actually the problem; it's the correct behavior in that case, where the aggregate start isn't being interpreted.)
-
If the comment precedes a non-aggregate parameter, the reader seems to skip that parameter entirely. Later parameters appear to be read just fine, however (presumably unless another troublesome comment is encountered).
To Reproduce
Demo code:
import sys
import pvl
def main():
if len(sys.argv) == 1:
print(file=sys.stderr)
print("A PVL filename is required.", file=sys.stderr)
print(file=sys.stderr)
exit(10)
pvl_filename = sys.argv[1]
print()
print(f"Trying to read {pvl_filename}...")
try:
pvl_file = pvl.load(pvl_filename)
except Exception as e:
print(f"Error loading {pvl_filename}:", file=sys.stderr)
print(e, file=sys.stderr)
exit(10)
print("Done.")
print()
print("Expected parameters:")
print()
print_param(pvl_file, "Instrument/FOV")
print_param(pvl_file, "Instrument/Filters")
print_param(pvl_file, "Logging")
print_param(pvl_file, "Engineering_Mode")
print()
def print_param(pvl_obj, param):
if "/" in param:
group, agg_param = param.split("/", 1)
if pvl_obj.get(group) == None:
value = None
else:
value = pvl_obj.get(group).get(agg_param)
else:
value = pvl_obj.get(param)
print(f"{param} = {value}")Base example PVL:
# the day the circus of heaven came into town...
Group = Instrument
FOV = 1.14
Filters = { RED, BLU, NIR }
End_Group
# local folk lined the streets in a midwestern town...
Logging = true
Engineering_Mode = false
The above PVL loads in the demo code just fine, producing the expected output:
Instrument/FOV = 1.14
Instrument/Filters = frozenset({'BLU', 'RED', 'NIR'})
Logging = True
Engineering_Mode = False
If one adds a dash (-) at the end of the first comment ("circus of heaven" comment), the demo code exits with the following exception message:
(LexerError(...), 'Expecting an Aggregation Block, an Assignment Statement, or an End Statement, but found "End_Group" : line 4 column 1 (char 116) near "BLU, NIR }\nEnd_Group\n\n#"')
If one instead adds a dash at the end of the second comment ("midwestern town" comment), the demo code produces the following output:
Instrument/FOV = 1.14
Instrument/Filters = frozenset({'RED', 'BLU', 'NIR'})
Logging = None
Engineering_Mode = False
Note here the Logging parameter is reported as having no value, while the Engineering_Mode parameter is reported with its correct value.
Expected behavior
I expect all three variants to produce the same results:
Instrument/FOV = 1.14
Instrument/Filters = frozenset({'RED', 'NIR', 'BLU'})
Logging = True
Engineering_Mode = False
There's of course an easy work-around, the doc-my-elbow-hurts-when-I-do-this solution: don't do that. But it does seem like this is unexpected behavior.
My Environment
- OS: macOS 15.6 Sequoia
- Python 3.9 and Python 3.13.7
- PVL version 1.3.2
Additional context
I only checked the Omni reader. The pvl_validate tool indicates the ISIS reader has the same problem with this kind of comment when it precedes the aggregate parameter, but of course it can't check the other variant of the problem (preceding a regular parameter) because no exception is thrown. The other readers just fail on the comment, period, with or without the ending dash, but I believe that's expected.
Admittedly I've been using PVL for 30-ish years now without reading any of the blue or orange books or what-have-you. Our own in-house Java tools allow this dash-ended-hashmark-comment just fine, however, and that developer definitely did read the books, as Ross of course knows...