IAU-ADES / ADES-Master

ADES implementation based on a master XML file
26 stars 7 forks source link

psvtoxml seems to be hardcoded to require version 2017 #8

Closed matthewjohnpayne closed 2 years ago

matthewjohnpayne commented 2 years ago

The psvtoxml script seems to be hardcoded to require version 2017 in the input file. Hence when presented with a "version=2022" input file it barfs, returning the error

Suggest changing this to allow version=2022

See code snippet below from the psvtoxml script ...

`# first non-blank line must be "#version=2017" #

stuff the attribute into the first node here as well

so we can have all this in one place if we want to

find things in the future.

# if (firstLine): firstLine = False l = line.split('=') if len(l) != 2: raise RuntimeError("first line of PSV must be '#version=2017'") #

strip all white space

  #
  if (''.join(l[0].split()) != '#version') or (''.join(l[1].split()) != '2017'):
    raise RuntimeError("first line of PSV must be '#version=2017'")

  # root node has no text or tail and one attribute
  stack = adesutility.ElementStack('ades', None, {'version':'2017'} )
  return

`

stevechesley commented 2 years ago

I'd suggest just dropping the version checking. So keep the first part of the test and drop the second. Like so:

if (''.join(l[0].split()) != '#version') :
     raise RuntimeError("first line of PSV must specify version, e.g., '#version=2017'")

As it stands, psvtoxml does not do any ADES validation. It just does a naive translation to XML, which must then be validated. That seems a better approach to validating the version number, rather than in psvtoxml.

stevechesley commented 2 years ago

psvtoxml is now agnostic as to what version it finds, just copying that value into the XML. But the version tag must still be present on first line of PSV.