DM-41922: Add datatypes and lengths to ObsCore #248
Conversation
|
@JeremyMcCormick I'm happy for you to take over this branch now. This is sufficient for my testing in dax_obscore. |
JeremyMcCormick
force-pushed
the
tickets/DM-41922
branch
from
11ffd16 to
c96bb5a
Compare
JeremyMcCormick
force-pushed
the
tickets/DM-41922
branch
from
c96bb5a to
1ac254a
Compare
|
@timj Jeremy and I were looking at this just now and trying to understand your "failed validation" comment. We don't see it failing automated validation. We agree that the changes you made were appropriate, but we just want to understand what you meant, first. |
yml/ivoa/obscore_nominal.yaml
Outdated
| @@ -538,7 +656,8 @@ tables: | |||
| votable:utype: Char.ObservableAxis.Accuracy.StatError.Refval.value | |||
| tap:std: 1 | |||
| tap:principal: 0 | |||
| ivoa:unit: same units as the value of the o_unit attribute | |||
| ivoa:unit: | |||
There was a problem hiding this comment.
@stvoutsin / @JeremyMcCormick it's this change that fixes the validation error. "same units as..." is not a valid unit.
There was a problem hiding this comment.
Sorry. I meant @gpdf there....
There was a problem hiding this comment.
I understand exactly why this would fail a validation, but is this theoretical, or is it right now failing validation in some place we can't see?
There was a problem hiding this comment.
It wasn't theoretical in the sense that I couldn't use this yaml file in dax_obscore without everything breaking.
There was a problem hiding this comment.
(Granted I can't remember the details now because it was a while back)
There was a problem hiding this comment.
OK, that explains why we can't see an actual failure in logs.
There was a problem hiding this comment.
Do you attempt to use this actual file, not a copy of it, programmatically, from dax_obscore?
There was a problem hiding this comment.
At the moment there is a copy in dax_obscore. Once this is merged and combined with the recent change to use python package resources I will be able to delete the local copy and use this one.
JeremyMcCormick
force-pushed
the
tickets/DM-41922
branch
from
1ac254a to
4bbadee
Compare
There was a problem hiding this comment.
Three issues:
obs_creation_datewas given the wrong datatype (it's a civil time, not an MJD)- based on extensive work by @JeremyMcCormick , explicitly declaring the
votable:arraysizeshould rarely if ever be needed - @JeremyMcCormick has made Felis'
timestampdatatype work correctly, so it should be used for time stamps, not least because a future update to Felis will then automatically generate the currently missingxtype='timestamp'specifier in TAP_SCHEMA.
I will make all these changes and retrofit them to the ObsCore-YAML-creation-from-the-standard script.
yml/ivoa/obscore_nominal.yaml
Outdated
| @@ -305,6 +365,7 @@ tables: | |||
| tap:std: 1 | |||
| tap:principal: 1 | |||
| ivoa:unit: | |||
| datatype: double | |||
There was a problem hiding this comment.
This is an incorrect data type for this column.
JeremyMcCormick
force-pushed
the
tickets/DM-41922
branch
from
4bbadee to
8ee0eb1
Compare
Defaults string lengths based on dax_obscore. Removed bad unit from o_stat_errror
| @@ -305,6 +365,7 @@ tables: | |||
| tap:std: 1 | |||
| tap:principal: 1 | |||
| ivoa:unit: | |||
| datatype: double | |||
There was a problem hiding this comment.
What is the data type for a date?
|
@JeremyMcCormick I've rebased and I'm failing to understand the deepdiff action that is failing. |
| @@ -17,6 +17,9 @@ tables: | |||
| tap:std: 1 | |||
| tap:principal: 1 | |||
| ivoa:unit: | |||
| datatype: string | |||
| votable:arraysize: "*" | |||
There was a problem hiding this comment.
Is the idea that because of some other code changes since I made this PR I can delete all the votable:arraysize lines?
Defaults string lengths based on dax_obscore.
Fails validation of o_stat_error because the unit is none standard.