Time series: Annotation of light curves in VOTable

[ada nebot]

Hi Mark, 

Thanks a lot for all your comments and the careful reading of the document. 

I’ve read all your comments and agree with your suggestions. I particularly 
like the controlled vocabulary for the magnitude system — I need to check 
a few things. 

I think I’ve gone through most of your suggestions, but I still leave some things 
open. Comments inline.  

> On 4 May 2020, at 17:54, Mark Taylor <m.b.taylor at bristol.ac.uk> wrote:
> 
> Ada and TDIG,
> 
> thanks for this Note.
> 
> On the whole I agree with the approach: use a GROUP rather than a
> new FILTERSYS element, on the grounds that it's best not to keep
> adding new elements to the VOTable schema.  GROUP is sufficient
> to do the job, and as long as the conventions are properly documented
> then clients should be able to understand this information.
> 

That’s ok. If this was to change I think it would be straight forward to 
make changes in the document. 

> If time series data encoded like this were fed to TOPCAT/STILTS
> as they stand, those applications wouldn't make much sense of them:
> they would not be able to present the photometry information to
> the user, and a save/load round trip of such VOTables would lose
> this metadata.  But I could add code to work with this information
> if we decide we want to use a convention like this.
> 

I think it would be very useful for users if applications such as TOPCAT/Stilts 
would read this GROUPs. We can discuss about particular functionalities, 
but just reading and plotting would already be a good start. 

> IMHO quite a bit of editorial work has to be done on the text.
> Some specific comments are below; if it helps I could make
> some edits and submit a PR or whatever.

Thanks for the suggestions, I agree, the text needs some editorial changes. 
You can’t believe how much making the PR would help me! 
I’ve made some modifications already (done) and leave some for you. 
Please, go ahead, make a PR and I will revise the modifications and merge. 

> - It looks like there are some leftovers from earlier approaches
>   in the text, e.g. "We propose ... define a set of new elements"
>   (sec 3) - in fact no new elements are defined.
> 

Done

> - There is some confusing nomenclature in sec 3.1, e.g. use of
>   "attribute" where "element" would be more accurate/clear.
> 

Yes, I get a bit mixed between attributes and elements… 
I let that to you. 

> - filterIdentifier description: 'Value SHOULD follow the syntax:
>   Facility/Subcategory/Band (e. g. value="GAIA/GAIA.G/G",
>   value="Palomar/ZTF.r").'
>     - the examples don't seem to follow the prescription,
>       e.g. "Palomar/ZTF.r" only has two elements not 3.
>       Fix or clarify?

Interesting, this is my mistake… It should be: 
Facility/Subcategory.Band 
Gaia/Gaia.G
and Palomar/ZTF.r

Done. 


> - Attribute descriptions in sec 3.1:
>   Items such as
>       datatype="datatype"
>       value="value"
>       unit="unit"
> 
>   are confusing; presumably e.g. "unit" here is a placeholder,
>   but it looks like a literal.  Possiblly rephrase like:
> 
>       unit=<unit>
> 

Yes, it is a placeholder, I used the latex \attrval{unit}{unit} … 

I changed to \attrval{unit}{<unit>}, now it’s 
      value=“<value>"
which is not what we want.. do you know how to change that? 

I leave that to you.  
 
>   But I'd be inclined to omit description of entries whose value is
>   not specific to this Note, e.g. unit, datatype.
> 

I put them there for completeness but I have nothing against removing them. 
In some cases I might have already removed them. Might need a check again. 

> - magnitudeSystem - should an associated Vocabulary be defined as for
>   e.g. TIMESYS timescale?
> 

This is a very good point. I need to look a bit into it, but I like the idea of having 
a controlled vocabulary for it.  

For now I have not changed the text, but I keep it in mind and not to forget it I have 
opened an issue on that topic. 

> - effectiveWavelength - the datatype is specified as "float".
>   Don't do this, since "double" is equally appropriate.
>   I would just avoid specifying datatype here, if you like
>   specify instead in the text that it's numeric.
>   Similarly for filterIdentifier and magnitudeSystem, I would
>   just specify in the text that they are strings - you don't
>   want to get into char vs. unicodeChar or have to talk about
>   arraysize.

OK, agree. Again here the units have been replaced… but can’t get rid of the '' 

I leave that to you.  
 
> - Sec 3.1 example:
>     - ucd="phot" looks reasonable, but is not mentioned earlier
>       in the section.  If this is recommended, talk about it earlier?
> 

Done

>     - unit="" - I don't think this is good practice.  If the quantity
>       doesn't have a unit, just omit this attribute.
> 
Done

>     - filterIdentifier and magnitudeSystem PARAMs have no arraysize
>       attributes.  Add arraysize="*" for PARAMs/FIELDs with
>       datatype="char" (unless you want to specify a fixed length).
> 

I guess after removing the datatype=“char” this is no longer needed.

> - <FILTERSYS> example at the end of section 3.1: maybe make it clearer
>   to casual readers that this is *not* the recommendation here, e.g.
> 
>      <!-- Rejected idea -->
>      <FILTERSYS ID="phot_sys" filterIdentifier="Palomar/ZTF.g/Vega"
>                 …/>
> 
Done

> - Sec 3.2: "One TABLE element MUST be present for each PHOTCAL
>   GROUP element defined." - Why?  It's probably good practice not
>   to litter the document with unused PHOTCALs, but I don't see
>   the point of outlawing them, and it suggests a 1:1 relationship
>   between PHOTCAL and TABLE which does not exist.
> 

Changed to "One or more… per TABLE“ - that was clearly contradictory. 
Thanks.

> - dataprocuct_type/dataproduct_subtype - again, I wouldn't mention
>   the datatype and arraysize attributes here (e.g. some authors
>   might want to replace arraysize="*" with arraysize="16"),
>   just say they are string-valued.
> 

Agreed, its not needed. Dropped. Done. 

> - Section 4.1 example: How about replacing the TIMESYS element here
>   with a more realistic/useful example?  If for no other reason than
>   that Arnold will chase you with a big stick for using HJD.
> 

> - Section 4.2 example: How about including (a few) data rows in
>   each table here rather than omitting them?  Adding e.g. three
>   rows to each table would not significantly increase the length
>   of the example XML and it would make it possible to cut'n'paste
>   this example in an application to see whether it really works.
> 

Done. 

Thanks! 

Cheers,
Ada

> Mark
> 
> 
> On Tue, 7 Apr 2020, ada nebot wrote:
> 
>> Dear IVOA members, 
>> 
>> We have drafted an IVOA Note on 
>> Time series: Annotation of light curves in VOTable
>> 
>> You can find this document under: 
>> http://www.ivoa.net/documents/Notes/LightCurveTimeSeries <http://www.ivoa.net/documents/Notes/LightCurveTimeSeries>
>> 
>> This document describes a proposal to annotate in a VOTable time series data. It is limited to the most common type of time series in astronomy: light curves, but it can be extended to other type of time series data easily (such as e. g. radial velocities curves). The annotation reuses elements of existing data models when possible and defines a set of new elements. 
>> 
>> Looking forward to your comments and suggestions! Also, don’t hesitate to distribute it around and ask us if you would like to be among the list of authors. We would be happy to add anyone with an interest on the proposal.  
>> 
>> Best,
>> Ada for the TDIG 
>> 
>> --
>> Astronome Adjointe
>> CDS, Observatoire Astronomique de Strasbourg (ObAS)
>> UMR 7550 Université de Strasbourg 
>> 11, rue de l'Université, F-67000 Strasbourg
>> +33 (0) 3 68 85 24 20
>> 
>> 
> 
> --
> Mark Taylor   Astronomical Programmer   Physics, Bristol University, UK
> m.b.taylor at bris.ac.uk +44-117-9288776  http://www.star.bris.ac.uk/~mbt/