search

open-gamma-ray-astro / gamma-astro-data-formats

Data formats for gamma-ray astronomy
https://gamma-astro-data-formats.readthedocs.io
Creative Commons Attribution 4.0 International
29 stars 27 forks source link

Improve and de-duplicate time specifications #103

Closed cdeil closed 6 years ago

cdeil commented 6 years ago

This pull request improves and avoids duplication in the specifications of times.

The only change in format is for the observation index files, where "DATE_OBS" with an underscore was used, whereas e.g. for the events "DATE-OBS" with a minus was used. My understanding is that this was a mistake, the FITS standard and also OGIP documents I found use "DATE-OBS" with a minus. I don't think that change is controversial, we already said in #95 that we want to use the same format for the observation index file that we use for EVENTS, and also almost no-one except us likes or wants to use the obs index tables anyways (see #7)

Another place where I saw "DATE_OBS" with a minus is in the CTA 1DC files produced by ctobssim, which I've reported at https://forge.in2p3.fr/boards/243/topics/2427 (cc @jknodlseder).

Apart from that small change / mistakes, the big change here in the spec is a simpler and rewritten general/time.rst page, and to reference that from other specs that contain times. Currently we had duplicated and slightly inconsistent and wrong descriptions four times (EVENTS, GTI, POINTING, OBSERVATION_INDEX), and getting all descriptions complete and correct and consistent is almost impossible if it's copy & pasted four times.

@jknodlseder @cboisson @TarekHC @lmohrmann - If you have time, please review this PR and leave a comment.

lmohrmann commented 6 years ago

@cdeil I don't have time to look at this in detail, but I'm certainly in favor of cleaning up the docs a bit. Your proposed changes look fine to me at first sight (but, as I said, I haven't looked at this in detail).

cdeil commented 6 years ago

Anyone else wants to review this? If not I'll merge this in the next days.

cdeil commented 6 years ago

@TarekHC - Thanks for the review. I've done some improvements in f86f7ab .

One suggestion I didn't follow was to add a subsection named "header keywords" to the section "Time format", because it didn't seem useful to have a more complex header structure for that page and to have a section with a single sub-section.

I did add a little more information on columns and header keys.

I'm merging this now.

If anyone finds an issue with this description, or has a suggestion to make it better, please leave a comment here, or better yet: open a pull request.