ivoa-std / SSA

Simple Spectral Access protocol
0 stars 1 forks source link

Fix issues reported in the operations 2022Oct session #10

Closed jd-au closed 1 year ago

jd-au commented 1 year ago

PDF preview of the revised document: SSA-WD-20230417.pdf

mbtaylor commented 1 year ago

@jd-au thank you for addressing these items. Since they are incorporated into the text rather than documented as separate Erratum items, is the intention to issue a new SSA v1.2 including these fixes?

Mostly the changes look good but one or two quibbles:

  1. Related to the clarification about the ssa: prefix, three of the utypes in the last example from Appendix A (page 57) are lacking this prefix, so it should be added to the example:

    <FIELD name="FluxAxis" utype="Dataset.FluxAxis" datatype="char" arraysize="*"> ...
    <FIELD name="SpectralSI" utype="Dataset.SpectralSI" datatype="char" arraysize="*"> ...
    <FIELD name="FluxSI" utype="Dataset.FluxSI" datatype="char" arraysize="*"> ...
  2. Concerning the MAN->REC changes for entries in section 4.2.5.10: the five utypes that are no longer MAN should I guess no longer be in bold face in the table.

  3. The removed xmlns:ssa declarations look fine. But nearby at the top of the example in Appendix B I see the crazy declaration

    xsi:noNamespaceSchemaLocation="xmlns:http://www.ivoa.net/xml/VOTable/VOTable-1.1.xsd"

    I'm pretty sure the "xmlns:" should be dropped from that attribute value. Maybe the best thing is just to stop looking at these examples...

msdemlei commented 1 year ago

Ummm... I feel a bit stupid... Is there a good way to look at the changes? I don't think github is smart (or rather, crazy) enough to diff word files, and at least libreoffice doesn't see any document-level changes in what I can check out from github.

On Mark's third point: Yes, the xmlns there is madness. I've not looked at this noNamespaceSchemaLocation thingy, but I'm pretty sure it won't save these VOTables from not validating as VOTable (even if they validated with XSD, which they don't either). All the examples should instead uniformly just use

  <VOTABLE xmlns="http://www.ivoa.net/xml/VOTable/v1.3" version="1.4">

(plus possibly declarations for xsi, but I don't think any of these have a good reason to declare xsi). Of course, when one actually tries to validate the examples, you get all kinds of funny errors, starting with

attribute 'type': [facet 'enumeration'] The value 'Results' is not an element of the set {'results', 'meta'}

(because you can't have type="Results").

Well, it's all I'd expect from a spec written in MS Word: https://xkcd.com/1301/ (and both doc and xls is ranked a lot too high there as far as I am concerned).

I guess what I'm saying is: Please go ahead, but I'm afraid if I start fiddling with SSA there'd not be left much of it... Perhaps patching the worst parts with Errata is the more promising way to go?

jd-au commented 1 year ago

Thanks for the feedback Mark and Markus.

I've updated all examples to use

<VOTABLE xmlns="http://www.ivoa.net/xml/VOTable/VOTable-1.1.xsd" version="1.1">

I wasn't comfortable with updating the VOTABLE version in this minimal update!

As the changes are a bit fiddly I was planning on putting out a SSA v1.2 revision rather than a bunch of errata. And as we are expecting future spectra support to be picked up by SIA1/DAP I've stuck with Word rather than migrating to tex. Of course we can still go with the errata if that is preferred.

I agree word is horrid for diffs - even the XML formats (odt, docx) have a mess of binary characters. Instead I have generated a diff using Microsoft Word and exported it as a PDF.

SSA-with-changes-PR10.pdf

mbtaylor commented 1 year ago

Thanks for the diffs. I think it's OK. As Markus says there are a number of other things you could imagine doing to it if you were doing a proper revision, but let's not. I was expecting Errata rather than a new version because of the additional effort of authoring and approving a new version and because the changes are small, but if you're happy pushing through a new version it's fine by me - hopefully people can appreciate that the changes are very minor and be persuaded to rubber stamp it quickly.