search

ivoa-std / VOTable

VOTable Format Definition
4 stars 15 forks source link

Vocabularising COOSYS/@system. #42

Closed msdemlei closed 11 months ago

msdemlei commented 12 months ago

This is supposed to address bug #22.

msdemlei commented 11 months ago

On Fri, Sep 01, 2023 at 03:15:09PM -0700, Tom wrote:

With this change to refer to the vocabulary document, some of the existing values have changed their case and here added the _.

Ah, no, that's not the intention. If you look at the actual vocabulary, https://www.ivoa.net/rdf/refframe, all legacy terms are still there. They are deprecated, however, in order to unify VOTable and Coords identifiers, and the ivoatex vocabulary machinery does not show deprecated terms (incidentally, I don't think it should).

While it does say below that clients should handle unknown frames gracefully, for backward compatibility should we give a specific recommendation on how to handle the listed values from VOTable 1.4?

As clients move to VOTable 1.5, the hope is they'll pull the vocabulary (most likely in its desise flavour), in which case they can do the old -> new translation based on the useInstead relations.

Perhaps that those values should continue to be supported but should be considered deprecated? We could recommend

I'll add some language stating that that is indeed what happens here.

tomdonaldson commented 11 months ago

On Fri, Sep 01, 2023 at 03:15:09PM -0700, Tom wrote: With this change to refer to the vocabulary document, some of the existing values have changed their case and here added the _. Ah, no, that's not the intention. If you look at the actual vocabulary, https://www.ivoa.net/rdf/refframe, all legacy terms are still there. They are deprecated, however, in order to unify VOTable and Coords identifiers, and the ivoatex vocabulary machinery does not show deprecated terms (incidentally, I don't think it should). While it does say below that clients should handle unknown frames gracefully, for backward compatibility should we give a specific recommendation on how to handle the listed values from VOTable 1.4? As clients move to VOTable 1.5, the hope is they'll pull the vocabulary (most likely in its desise flavour), in which case they can do the old -> new translation based on the useInstead relations. Perhaps that those values should continue to be supported but should be considered deprecated? We could recommend I'll add some language stating that that is indeed what happens here.

Ah good. Thanks for clarifying that. Adding language to that effect in the doc may help others who have the same misunderstanding I did.

mbtaylor commented 11 months ago

Note: to avoid confusion (mine at least) I have renamed this PR from "Vocabularising COOSYS/@refposition" to "Vocabularising COOSYS/@system", since the original title was incorrect (it referred to work done in #33).

mbtaylor commented 11 months ago

VOTable.xsd still says

      <xs:attribute name="system" default="eq_FK5">

i.e. it declares the default of the system attribute to be a now-deprecated value. This should presumably be changed; refframe declares FK5 as the useInstead for eq_FK5, but maybe ICRS would be better?

msdemlei commented 11 months ago

On Wed, Sep 06, 2023 at 10:45:11AM -0700, Tom wrote:

Ah good. Thanks for clarifying that. Adding language to that effect in the doc may help others who have the same misunderstanding I did.

With the last commit, I had introduced:

Until VOTable 1.4, these identifiers were defined in the VOTable schema, and some systems had different identifiers. These legacy identifiers are still in the vocabulary, but they are deprecated.

Do you have suggestions for how to improve that?

Meanwhile, I noticed I haven't put in a changelog entry, so I overwrite (push -f) the last commit. If you have a checkout of vocabularise-coosys-system, please discard it before pulling it again.

msdemlei commented 11 months ago

On Thu, Sep 07, 2023 at 05:06:46AM -0700, Mark Taylor wrote:

VOTable.xsd still says

      <xs:attribute name="system" default="eq_FK5">

i.e. it declares the default of the system attribute to be a now-deprecated value.

Aw, right. Dang. See commit 0511720c for which we can't move to ICRS (regrettably, but then people shouldn't rely on defaults for fundamental metadata like that anyway).