planetarypy / pvl

Python implementation of PVL (Parameter Value Language)
BSD 3-Clause "New" or "Revised" License
19 stars 19 forks source link

pvl.grammar objects have incorrect documentation, and ISISGrammar needs to handle #-comments #73

Closed rbeyer closed 3 years ago

rbeyer commented 3 years ago

Describe the bug The documentation for the various grammar objects in grammar.py incorrectly conveys that there are some parameters that could be specified on object instantiation. Additionally, the ISISGrammar needs to be able to handle octothorpe (#) started comments, and it does not.

Expected behavior

Additional context Please see #72 for a more extended discussion related to these topics.

rbeyer commented 3 years ago

@acpaquette, as I was working on this, something seemed off. In #72, you indicated that you were having issues because the ISISGrammar couldn't deal with octothorpe-comments, and the OmniParser couldn't deal with plus (+) characters in Unquoted String values. At the time, I was having computer problems, and couldn't actually check this.

Your example PVL-text doesn't have unquoted plus signs, but in the pvl test data, there is an isis_naif.txt which does. When I check it with pvl_validate I get this:

> pvl_validate tests/data/isis_naif.txt
PDS3 | does NOT load |
ODL  | does NOT load |
PVL  | does NOT load |
ISIS |     Loads     |     Encodes
Omni |     Loads     |     Encodes

This indicates that both the ISISGrammar and OmniGrammar should be able to parse unquoted plus signs.

If I take your example PVL-text from #72 (and add an End_Group line):

> pvl_validate octothorpe.txt
PDS3 | does NOT load |
ODL  | does NOT load |
PVL  | does NOT load |
ISIS | does NOT load |
Omni |     Loads     |     Encodes

This indicates that the OmniGrammar loads it just fine. That doesn't change anything in particular, as the ISISGrammar should parse this kind of comment too, but I guess I'm curious why you thought that the OmniGrammar wasn't parsing it, just in case there's some other nefarious bug lurking about.